* [RFC net-next 01/15] net: cpc: add base skeleton driver
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 2:13 ` Andrew Lunn
2025-05-12 1:27 ` [RFC net-next 02/15] net: cpc: add endpoint infrastructure Damien Riégel
` (14 subsequent siblings)
15 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
This commit prepares the addition of a CPC driver. CPC, standing for
Co-Processor Communication, enables users to have multiple stack
protocols over a shared physical link using multiple endpoints.
This patch adds the basic infrastructure for the new module, and
introduces a new structure `cpc_interface`. The goal of this structure
is to abstract a physical link like an SPI device, a SDIO function, or a
UART for instance.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
MAINTAINERS | 6 +++
drivers/net/Kconfig | 2 +
drivers/net/Makefile | 1 +
drivers/net/cpc/Kconfig | 15 ++++++
drivers/net/cpc/Makefile | 5 ++
drivers/net/cpc/interface.c | 98 +++++++++++++++++++++++++++++++++++++
drivers/net/cpc/interface.h | 88 +++++++++++++++++++++++++++++++++
drivers/net/cpc/main.c | 21 ++++++++
8 files changed, 236 insertions(+)
create mode 100644 drivers/net/cpc/Kconfig
create mode 100644 drivers/net/cpc/Makefile
create mode 100644 drivers/net/cpc/interface.c
create mode 100644 drivers/net/cpc/interface.h
create mode 100644 drivers/net/cpc/main.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 00e94bec401..8256ec0ff8a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21731,6 +21731,12 @@ S: Maintained
F: drivers/input/touchscreen/silead.c
F: drivers/platform/x86/touchscreen_dmi.c
+SILICON LABS CPC DRIVERS
+M: Damien Riégel <damien.riegel@silabs.com>
+R: Silicon Labs Kernel Team <linux-devel@silabs.com>
+S: Supported
+F: drivers/net/cpc/*
+
SILICON LABS WIRELESS DRIVERS (for WFxxx series)
M: Jérôme Pouiller <jerome.pouiller@silabs.com>
S: Supported
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 1fd5acdc73c..d78ca2f4de5 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -508,6 +508,8 @@ source "drivers/atm/Kconfig"
source "drivers/net/caif/Kconfig"
+source "drivers/net/cpc/Kconfig"
+
source "drivers/net/dsa/Kconfig"
source "drivers/net/ethernet/Kconfig"
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 13743d0e83b..19878d11c62 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_MHI_NET) += mhi_net.o
obj-$(CONFIG_ARCNET) += arcnet/
obj-$(CONFIG_CAIF) += caif/
obj-$(CONFIG_CAN) += can/
+obj-$(CONFIG_CPC) += cpc/
ifdef CONFIG_NET_DSA
obj-y += dsa/
endif
diff --git a/drivers/net/cpc/Kconfig b/drivers/net/cpc/Kconfig
new file mode 100644
index 00000000000..f31b6837b49
--- /dev/null
+++ b/drivers/net/cpc/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+
+menuconfig CPC
+ tristate "Silicon Labs Co-Processor Communication (CPC) Protocol"
+ depends on NET
+ help
+ Provide support for the CPC protocol to Silicon Labs EFR32 devices.
+
+ CPC provides a way to multiplex data channels over a shared physical
+ link. These data channels can carry Bluetooth, Wi-Fi, or any arbitrary
+ data. Depending on the part and the firmware, the set of available
+ channels may differ.
+
+ Say Y here to compile support for CPC into the kernel or say M to
+ compile as a module.
diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
new file mode 100644
index 00000000000..1ce7415f305
--- /dev/null
+++ b/drivers/net/cpc/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+cpc-y := interface.o main.o
+
+obj-$(CONFIG_CPC) += cpc.o
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
new file mode 100644
index 00000000000..4fdc78a0868
--- /dev/null
+++ b/drivers/net/cpc/interface.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/module.h>
+
+#include "interface.h"
+
+#define to_cpc_interface(d) container_of(d, struct cpc_interface, dev)
+
+static DEFINE_IDA(cpc_ida);
+
+/**
+ * cpc_intf_release() - Actual release of interface.
+ * @dev: Device embedded in struct cpc_interface
+ *
+ * This function should not be called directly, users are expected to use cpc_interface_put()
+ * instead. This function will be called when the last reference to the CPC device is released.
+ */
+static void cpc_intf_release(struct device *dev)
+{
+ struct cpc_interface *intf = to_cpc_interface(dev);
+
+ ida_free(&cpc_ida, intf->index);
+ kfree(intf);
+}
+
+/**
+ * cpc_interface_alloc() - Allocate memory for new CPC interface.
+ *
+ * @parent: Parent device.
+ * @ops: Callbacks for this device.
+ * @priv: Pointer to private structure associated with this device.
+ *
+ * Context: Process context as allocations are done with @GFP_KERNEL flag
+ *
+ * Return: allocated CPC interface or %NULL.
+ */
+struct cpc_interface *cpc_interface_alloc(struct device *parent,
+ const struct cpc_interface_ops *ops,
+ void *priv)
+{
+ struct cpc_interface *intf;
+
+ intf = kzalloc(sizeof(*intf), GFP_KERNEL);
+ if (!intf)
+ return NULL;
+
+ intf->index = ida_alloc(&cpc_ida, GFP_KERNEL);
+ if (intf->index < 0) {
+ kfree(intf);
+ return NULL;
+ }
+
+ intf->ops = ops;
+
+ intf->dev.parent = parent;
+ intf->dev.release = cpc_intf_release;
+
+ device_initialize(&intf->dev);
+
+ dev_set_name(&intf->dev, "cpc%d", intf->index);
+ dev_set_drvdata(&intf->dev, priv);
+
+ return intf;
+}
+
+/**
+ * cpc_interface_register() - Register CPC interface.
+ * @intf: CPC device to register.
+ *
+ * Context: Process context.
+ *
+ * Return: 0 if successful, otherwise a negative error code.
+ */
+int cpc_interface_register(struct cpc_interface *intf)
+{
+ int err;
+
+ err = device_add(&intf->dev);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+/**
+ * cpc_interface_unregister() - Unregister a CPC interface.
+ * @intf: CPC device to unregister.
+ *
+ * Context: Process context.
+ */
+void cpc_interface_unregister(struct cpc_interface *intf)
+{
+ device_del(&intf->dev);
+ cpc_interface_put(intf);
+}
diff --git a/drivers/net/cpc/interface.h b/drivers/net/cpc/interface.h
new file mode 100644
index 00000000000..797f70119a8
--- /dev/null
+++ b/drivers/net/cpc/interface.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_INTERFACE_H
+#define __CPC_INTERFACE_H
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/skbuff.h>
+
+struct cpc_interface;
+struct cpc_interface_ops;
+
+/**
+ * struct cpc_interface - Representation of a CPC interface.
+ * @dev: Device structure for bookkeeping..
+ * @ops: Callbacks for this device.
+ * @index: Device index.
+ */
+struct cpc_interface {
+ struct device dev;
+
+ const struct cpc_interface_ops *ops;
+
+ int index;
+};
+
+/**
+ * struct cpc_interface_ops - Callbacks from CPC core to physical bus driver.
+ * @wake_tx: Called by CPC core to wake up the transmit task of that interface.
+ * @csum: Callback to calculate checksum over the payload.
+ *
+ * This structure contains various callbacks that the bus (SDIO, SPI) driver must implement.
+ */
+struct cpc_interface_ops {
+ int (*wake_tx)(struct cpc_interface *intf);
+ void (*csum)(struct sk_buff *skb);
+};
+
+struct cpc_interface *cpc_interface_alloc(struct device *parent,
+ const struct cpc_interface_ops *ops,
+ void *priv);
+
+int cpc_interface_register(struct cpc_interface *intf);
+void cpc_interface_unregister(struct cpc_interface *intf);
+
+/**
+ * cpc_interface_get() - Get a reference to interface and return its pointer.
+ * @intf: Interface to get.
+ *
+ * Return: Interface pointer with its reference counter incremented, or %NULL.
+ */
+static inline struct cpc_interface *cpc_interface_get(struct cpc_interface *intf)
+{
+ if (!intf || !get_device(&intf->dev))
+ return NULL;
+ return intf;
+}
+
+/**
+ * cpc_interface_put() - Release reference to an interface.
+ * @intf: CPC interface
+ *
+ * Context: Process context.
+ */
+static inline void cpc_interface_put(struct cpc_interface *intf)
+{
+ if (intf)
+ put_device(&intf->dev);
+}
+
+/**
+ * cpc_interface_get_priv() - Get driver data associated with this interface.
+ * @intf: Interface pointer.
+ *
+ * Return: Driver data, set at allocation via cpc_interface_alloc().
+ */
+static inline void *cpc_interface_get_priv(struct cpc_interface *intf)
+{
+ if (!intf)
+ return NULL;
+ return dev_get_drvdata(&intf->dev);
+}
+
+#endif
diff --git a/drivers/net/cpc/main.c b/drivers/net/cpc/main.c
new file mode 100644
index 00000000000..ba9ab1ccf63
--- /dev/null
+++ b/drivers/net/cpc/main.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/module.h>
+
+static int __init cpc_init(void)
+{
+ return 0;
+}
+module_init(cpc_init);
+
+static void __exit cpc_exit(void)
+{
+}
+module_exit(cpc_exit);
+
+MODULE_DESCRIPTION("Silicon Labs CPC Protocol");
+MODULE_AUTHOR("Damien Riégel <damien.riegel@silabs.com>");
+MODULE_LICENSE("GPL");
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC net-next 01/15] net: cpc: add base skeleton driver
2025-05-12 1:27 ` [RFC net-next 01/15] net: cpc: add base skeleton driver Damien Riégel
@ 2025-05-12 2:13 ` Andrew Lunn
0 siblings, 0 replies; 39+ messages in thread
From: Andrew Lunn @ 2025-05-12 2:13 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
> +/**
> + * cpc_interface_register() - Register CPC interface.
> + * @intf: CPC device to register.
> + *
> + * Context: Process context.
> + *
> + * Return: 0 if successful, otherwise a negative error code.
> + */
> +int cpc_interface_register(struct cpc_interface *intf)
> +{
> + int err;
> +
> + err = device_add(&intf->dev);
> + if (err)
> + return err;
> +
> + return 0;
I guess this will change in a later patch, but maybe not. This should
just be
return device_add(&intf->dev);
> +}
> +
> +/**
> + * cpc_interface_unregister() - Unregister a CPC interface.
> + * @intf: CPC device to unregister.
> + *
> + * Context: Process context.
> + */
> +void cpc_interface_unregister(struct cpc_interface *intf)
> +{
> + device_del(&intf->dev);
> + cpc_interface_put(intf);
> +}
It seems odd that unregister is not a mirror of register?
> +/**
> + * cpc_interface_get() - Get a reference to interface and return its pointer.
> + * @intf: Interface to get.
> + *
> + * Return: Interface pointer with its reference counter incremented, or %NULL.
> + */
> +static inline struct cpc_interface *cpc_interface_get(struct cpc_interface *intf)
> +{
> + if (!intf || !get_device(&intf->dev))
> + return NULL;
> + return intf;
> +}
What is the use case for passing in NULL?
> +
> +/**
> + * cpc_interface_put() - Release reference to an interface.
> + * @intf: CPC interface
> + *
> + * Context: Process context.
> + */
> +static inline void cpc_interface_put(struct cpc_interface *intf)
> +{
> + if (intf)
> + put_device(&intf->dev);
> +}
> +
> +/**
> + * cpc_interface_get_priv() - Get driver data associated with this interface.
> + * @intf: Interface pointer.
> + *
> + * Return: Driver data, set at allocation via cpc_interface_alloc().
> + */
> +static inline void *cpc_interface_get_priv(struct cpc_interface *intf)
> +{
> + if (!intf)
> + return NULL;
> + return dev_get_drvdata(&intf->dev);
> +}
What is the use case for passing in NULL?
To me, this is hiding bugs. It seems better to let the kernel opp so
you find out where you lost your intf.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC net-next 02/15] net: cpc: add endpoint infrastructure
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
2025-05-12 1:27 ` [RFC net-next 01/15] net: cpc: add base skeleton driver Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 2:28 ` Andrew Lunn
2025-05-12 1:27 ` [RFC net-next 03/15] net: cpc: introduce CPC driver and bus Damien Riégel
` (13 subsequent siblings)
15 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Network stacks using CPC are isolated from each other and their
communication channels are called endpoints. Within a CPC interface,
endpoints must have a unique Endpoint ID, which will be used to address
messages to that specific endpoint in a latter changeset.
Endpoints are part of an interface, this is represented in the device
model by endpoints being children of interface, and the interface
ensuring uniqueness of the endpoint ID when a new one is added.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/Makefile | 2 +-
drivers/net/cpc/cpc.h | 101 ++++++++++++++++++++++
drivers/net/cpc/endpoint.c | 166 ++++++++++++++++++++++++++++++++++++
drivers/net/cpc/interface.c | 58 +++++++++++++
drivers/net/cpc/interface.h | 11 +++
5 files changed, 337 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/cpc/cpc.h
create mode 100644 drivers/net/cpc/endpoint.c
diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
index 1ce7415f305..673a40db424 100644
--- a/drivers/net/cpc/Makefile
+++ b/drivers/net/cpc/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-cpc-y := interface.o main.o
+cpc-y := endpoint.o interface.o main.o
obj-$(CONFIG_CPC) += cpc.o
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
new file mode 100644
index 00000000000..529319f4339
--- /dev/null
+++ b/drivers/net/cpc/cpc.h
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_H
+#define __CPC_H
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+#define CPC_ENDPOINT_NAME_MAX_LEN 128
+
+struct cpc_driver;
+struct cpc_interface;
+struct cpc_endpoint;
+
+/**
+ * struct cpc_endpoint - Representation of CPC endpointl
+ * @dev: Driver model representation of the device.
+ * @name: Endpoint name, used for matching with corresponding driver.
+ * @id: Endpoint id, uniquely identifies an endpoint within a CPC device.
+ * @intf: Pointer to CPC device this endpoint belongs to.
+ * @list_node: list_head member for linking in a CPC device.
+ *
+ * Each endpoint can send and receive data without consideration of the other endpoints sharing the
+ * same physical link.
+ */
+struct cpc_endpoint {
+ struct device dev;
+
+ char name[CPC_ENDPOINT_NAME_MAX_LEN];
+ u8 id;
+
+ struct cpc_interface *intf;
+ struct list_head list_node;
+};
+
+struct cpc_endpoint *cpc_endpoint_alloc(struct cpc_interface *intf, u8 id);
+int cpc_endpoint_register(struct cpc_endpoint *ep);
+struct cpc_endpoint *cpc_endpoint_new(struct cpc_interface *intf, u8 id, const char *ep_name);
+
+void cpc_endpoint_unregister(struct cpc_endpoint *ep);
+
+/**
+ * cpc_endpoint_from_dev() - Upcast from a device pointer.
+ * @dev: Reference to a device.
+ *
+ * Return: Reference to the cpc endpoint.
+ */
+static inline struct cpc_endpoint *cpc_endpoint_from_dev(const struct device *dev)
+{
+ return container_of(dev, struct cpc_endpoint, dev);
+}
+
+/**
+ * cpc_endpoint_get() - Get a reference to endpoint and return its pointer.
+ * @ep: Endpoint to get.
+ *
+ * Return: Endpoint pointer with its reference counter incremented, or %NULL.
+ */
+static inline struct cpc_endpoint *cpc_endpoint_get(struct cpc_endpoint *ep)
+{
+ if (!ep || !get_device(&ep->dev))
+ return NULL;
+ return ep;
+}
+
+/**
+ * cpc_endpoint_put() - Release reference to an endpoint.
+ * @ep: CPC endpoint, allocated by cpc_endpoint_alloc().
+ *
+ * Context: Process context.
+ */
+static inline void cpc_endpoint_put(struct cpc_endpoint *ep)
+{
+ if (ep)
+ put_device(&ep->dev);
+}
+
+/**
+ * cpc_endpoint_get_drvdata() - Get driver data associated with this endpoint.
+ * @ep: Endpoint.
+ *
+ * Return: Driver data, set by cpc_endpoint_set_drvdata().
+ */
+static inline void *cpc_endpoint_get_drvdata(struct cpc_endpoint *ep)
+{
+ return dev_get_drvdata(&ep->dev);
+}
+
+/**
+ * cpc_endpoint_set_drvdata() - Set driver data for this endpoint.
+ * @ep: Endpoint.
+ */
+static inline void cpc_endpoint_set_drvdata(struct cpc_endpoint *ep, void *data)
+{
+ dev_set_drvdata(&ep->dev, data);
+}
+
+#endif
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
new file mode 100644
index 00000000000..5aef8d7e43c
--- /dev/null
+++ b/drivers/net/cpc/endpoint.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/string.h>
+
+#include "cpc.h"
+#include "interface.h"
+
+/**
+ * cpc_ep_release() - Actual release of the CPC endpoint.
+ * @dev: Device embedded in struct cpc_endpoint.
+ *
+ * This function should not be called directly, users are expected to use cpc_endpoint_put().
+ */
+static void cpc_ep_release(struct device *dev)
+{
+ struct cpc_endpoint *ep = cpc_endpoint_from_dev(dev);
+
+ cpc_interface_put(ep->intf);
+ kfree(ep);
+}
+
+/**
+ * cpc_endpoint_alloc() - Allocate memory for new CPC endpoint.
+ * @intf: CPC interface owning this endpoint.
+ * @id: Endpoint ID.
+ *
+ * Context: Process context as allocations are done with @GFP_KERNEL flag
+ *
+ * Return: allocated CPC endpoint or %NULL.
+ */
+struct cpc_endpoint *cpc_endpoint_alloc(struct cpc_interface *intf, u8 id)
+{
+ struct cpc_endpoint *ep;
+
+ if (!cpc_interface_get(intf))
+ return NULL;
+
+ ep = kzalloc(sizeof(*ep), GFP_KERNEL);
+ if (!ep) {
+ cpc_interface_put(intf);
+ return NULL;
+ }
+
+ ep->intf = intf;
+ ep->id = id;
+
+ ep->dev.parent = &intf->dev;
+ ep->dev.release = cpc_ep_release;
+
+ device_initialize(&ep->dev);
+
+ return ep;
+}
+
+static int cpc_ep_check_unique_id(struct device *dev, void *data)
+{
+ struct cpc_endpoint *ep = cpc_endpoint_from_dev(dev);
+ struct cpc_endpoint *new_ep = data;
+
+ if (ep->id == new_ep->id)
+ return -EBUSY;
+
+ return 0;
+}
+
+static int __cpc_endpoint_register(struct cpc_endpoint *ep)
+{
+ size_t name_len;
+ int err;
+
+ name_len = strnlen(ep->name, sizeof(ep->name));
+ if (name_len == 0 || name_len == sizeof(ep->name))
+ return -EINVAL;
+
+ err = dev_set_name(&ep->dev, "%s.%d", dev_name(&ep->intf->dev), ep->id);
+ if (err) {
+ dev_err(&ep->dev, "failed to dev_set_name (%d)\n", err);
+ return err;
+ }
+
+ err = device_for_each_child(&ep->intf->dev, ep, cpc_ep_check_unique_id);
+ if (err)
+ return err;
+
+ err = device_add(&ep->dev);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+/**
+ * cpc_endpoint_register() - Register an endpoint.
+ * @ep: Endpoint to register.
+ *
+ * Companion function of cpc_endpoint_alloc(). This function adds the endpoint, making it usable by
+ * CPC drivers. As this ensures that endpoint ID is unique within a CPC interface and then adds the
+ * endpoint, the lock interface is held to prevent concurrent additions.
+ *
+ * Context: Lock "add_lock" of endpoint's interface.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int cpc_endpoint_register(struct cpc_endpoint *ep)
+{
+ int err;
+
+ if (!ep || !ep->intf)
+ return -EINVAL;
+
+ mutex_lock(&ep->intf->add_lock);
+ err = __cpc_endpoint_register(ep);
+ mutex_unlock(&ep->intf->add_lock);
+
+ return err;
+}
+
+/**
+ * cpc_endpoint_new() - Convenience wrapper to allocate and register an endpoint.
+ * @intf: The interface the endpoint will be attached to.
+ * @id: ID of the endpoint to add.
+ * @ep_name: Name of the endpoint to add.
+ *
+ * Context: Process context, as allocation are done with GFP_KERNEL and interface's lock is
+ * acquired.
+ *
+ * Return: Newly added endpoint, or %NULL in case of error.
+ */
+struct cpc_endpoint *cpc_endpoint_new(struct cpc_interface *intf, u8 id, const char *ep_name)
+{
+ struct cpc_endpoint *ep;
+ int err;
+
+ ep = cpc_endpoint_alloc(intf, id);
+ if (!ep)
+ return NULL;
+
+ if (ep_name)
+ strscpy(ep->name, ep_name);
+
+ err = cpc_endpoint_register(ep);
+ if (err)
+ goto put_ep;
+
+ return ep;
+
+put_ep:
+ cpc_endpoint_put(ep);
+
+ return NULL;
+}
+
+/** cpc_endpoint_unregister() - Unregister an endpoint.
+ * @ep: Endpoint registered with cpc_endpoint_new() or cpc_endpoint_register().
+ *
+ * Unregister an endpoint, its resource will be freed when the last reference to this
+ * endpoint is dropped.
+ */
+void cpc_endpoint_unregister(struct cpc_endpoint *ep)
+{
+ device_del(&ep->dev);
+ put_device(&ep->dev);
+}
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
index 4fdc78a0868..6b3fc16f212 100644
--- a/drivers/net/cpc/interface.c
+++ b/drivers/net/cpc/interface.c
@@ -5,6 +5,7 @@
#include <linux/module.h>
+#include "cpc.h"
#include "interface.h"
#define to_cpc_interface(d) container_of(d, struct cpc_interface, dev)
@@ -53,6 +54,10 @@ struct cpc_interface *cpc_interface_alloc(struct device *parent,
return NULL;
}
+ mutex_init(&intf->add_lock);
+ mutex_init(&intf->lock);
+ INIT_LIST_HEAD(&intf->eps);
+
intf->ops = ops;
intf->dev.parent = parent;
@@ -85,6 +90,12 @@ int cpc_interface_register(struct cpc_interface *intf)
return 0;
}
+static int cpc_intf_unregister_ep(struct device *dev, void *null)
+{
+ cpc_endpoint_unregister(cpc_endpoint_from_dev(dev));
+ return 0;
+}
+
/**
* cpc_interface_unregister() - Unregister a CPC interface.
* @intf: CPC device to unregister.
@@ -93,6 +104,53 @@ int cpc_interface_register(struct cpc_interface *intf)
*/
void cpc_interface_unregister(struct cpc_interface *intf)
{
+ /* Iterate in reverse order so that system endpoint is removed last. */
+ device_for_each_child_reverse(&intf->dev, NULL, cpc_intf_unregister_ep);
+
device_del(&intf->dev);
cpc_interface_put(intf);
}
+
+/**
+ * __cpc_interface_get_endpoint() - get endpoint registered in CPC device with this id without lock
+ * @intf: CPC device to probe
+ * @ep_id: endpoint ID that's being looked for
+ *
+ * Get an endpoint by its ID if present in a CPC device. Endpoint's ref count is incremented and
+ * should be decremented with cpc_endpoint_put() when done.
+ *
+ * Context: This function doesn't lock device's endpoint list, caller is responsible for that.
+ *
+ * Return: a struct cpc_endpoint pointer or NULL if not found.
+ */
+static struct cpc_endpoint *__cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id)
+{
+ struct cpc_endpoint *ep_it;
+
+ list_for_each_entry(ep_it, &intf->eps, list_node) {
+ if (ep_it->id == ep_id)
+ return cpc_endpoint_get(ep_it);
+ }
+
+ return NULL;
+}
+
+/**
+ * cpc_interface_get_endpoint() - get endpoint registered in CPC device with this id
+ * @intf: CPC device to probe
+ * @ep_id: endpoint ID that's being looked for
+ *
+ * Context: This function locks device's endpoint list.
+ *
+ * Return: a struct cpc_endpoint pointer or NULL if not found.
+ */
+struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id)
+{
+ struct cpc_endpoint *ep;
+
+ mutex_lock(&intf->lock);
+ ep = __cpc_interface_get_endpoint(intf, ep_id);
+ mutex_unlock(&intf->lock);
+
+ return ep;
+}
diff --git a/drivers/net/cpc/interface.h b/drivers/net/cpc/interface.h
index 797f70119a8..d6b6d9ce5de 100644
--- a/drivers/net/cpc/interface.h
+++ b/drivers/net/cpc/interface.h
@@ -17,15 +17,24 @@ struct cpc_interface_ops;
/**
* struct cpc_interface - Representation of a CPC interface.
* @dev: Device structure for bookkeeping..
+ * @add_lock: Lock to serialize addition of new endpoints.
* @ops: Callbacks for this device.
* @index: Device index.
+ * @lock: Protect access to endpoint list.
+ * @eps: List of endpoints managed by this device.
*/
struct cpc_interface {
struct device dev;
+ /* Prevent concurrent addition of new devices */
+ struct mutex add_lock;
+
const struct cpc_interface_ops *ops;
int index;
+
+ struct mutex lock; /* Protect eps from concurrent access. */
+ struct list_head eps;
};
/**
@@ -47,6 +56,8 @@ struct cpc_interface *cpc_interface_alloc(struct device *parent,
int cpc_interface_register(struct cpc_interface *intf);
void cpc_interface_unregister(struct cpc_interface *intf);
+struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id);
+
/**
* cpc_interface_get() - Get a reference to interface and return its pointer.
* @intf: Interface to get.
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC net-next 02/15] net: cpc: add endpoint infrastructure
2025-05-12 1:27 ` [RFC net-next 02/15] net: cpc: add endpoint infrastructure Damien Riégel
@ 2025-05-12 2:28 ` Andrew Lunn
0 siblings, 0 replies; 39+ messages in thread
From: Andrew Lunn @ 2025-05-12 2:28 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
> +/**
> + * cpc_endpoint_register() - Register an endpoint.
> + * @ep: Endpoint to register.
> + *
> + * Companion function of cpc_endpoint_alloc(). This function adds the endpoint, making it usable by
> + * CPC drivers. As this ensures that endpoint ID is unique within a CPC interface and then adds the
> + * endpoint, the lock interface is held to prevent concurrent additions.
> + *
> + * Context: Lock "add_lock" of endpoint's interface.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int cpc_endpoint_register(struct cpc_endpoint *ep)
> +{
> + int err;
> +
> + if (!ep || !ep->intf)
> + return -EINVAL;
> +
> + mutex_lock(&ep->intf->add_lock);
> + err = __cpc_endpoint_register(ep);
> + mutex_unlock(&ep->intf->add_lock);
What exactly is add_lock protecting?
> +void cpc_endpoint_unregister(struct cpc_endpoint *ep)
> +{
> + device_del(&ep->dev);
> + put_device(&ep->dev);
> +}
Register needs a lock, but unregister does not?
> +/**
> + * cpc_interface_get_endpoint() - get endpoint registered in CPC device with this id
> + * @intf: CPC device to probe
> + * @ep_id: endpoint ID that's being looked for
> + *
> + * Context: This function locks device's endpoint list.
> + *
> + * Return: a struct cpc_endpoint pointer or NULL if not found.
> + */
> +struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id)
> +{
> + struct cpc_endpoint *ep;
> +
> + mutex_lock(&intf->lock);
> + ep = __cpc_interface_get_endpoint(intf, ep_id);
> + mutex_unlock(&intf->lock);
> +
> + return ep;
> +}
cpc_interface_get_endpoint() but no cpc_interface_put_endpoint() ? Is
this not taking a reference on the end point? Maybe this should not be
called _get_.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC net-next 03/15] net: cpc: introduce CPC driver and bus
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
2025-05-12 1:27 ` [RFC net-next 01/15] net: cpc: add base skeleton driver Damien Riégel
2025-05-12 1:27 ` [RFC net-next 02/15] net: cpc: add endpoint infrastructure Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 04/15] net: cpc: add protocol header structure and API Damien Riégel
` (12 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Endpoints by itself are useless if there are no drivers to use them.
This commit adds the final bit of infrastructure for CPC module: a new
bus type and its associated driver.
As a very basic matching mechanism, the bus will match an endpoint with
its driver if driver's name (driver.name attribute) matches endpoint's
name.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/cpc.h | 39 +++++++++++++++++++++++
drivers/net/cpc/endpoint.c | 1 +
drivers/net/cpc/main.c | 65 +++++++++++++++++++++++++++++++++++++-
3 files changed, 104 insertions(+), 1 deletion(-)
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
index 529319f4339..cbd1b3d6a03 100644
--- a/drivers/net/cpc/cpc.h
+++ b/drivers/net/cpc/cpc.h
@@ -15,6 +15,8 @@ struct cpc_driver;
struct cpc_interface;
struct cpc_endpoint;
+extern const struct bus_type cpc_bus;
+
/**
* struct cpc_endpoint - Representation of CPC endpointl
* @dev: Driver model representation of the device.
@@ -98,4 +100,41 @@ static inline void cpc_endpoint_set_drvdata(struct cpc_endpoint *ep, void *data)
dev_set_drvdata(&ep->dev, data);
}
+/*---------------------------------------------------------------------------*/
+
+/**
+ * struct cpc_driver - CPC endpoint driver.
+ * @driver: Internal driver for the device driver model.
+ * @probe: Binds this driver to the endpoint.
+ * @remove: Unbinds this driver from the endpoint.
+ *
+ * This represents a device driver that uses an endpoint to communicate with a remote application at
+ * the other side of the CPC interface. The way to communicate with the remote is abstracted by the
+ * interface, and drivers don't have to care if other endpoints are present or not.
+ */
+struct cpc_driver {
+ struct device_driver driver;
+
+ int (*probe)(struct cpc_endpoint *ep);
+ void (*remove)(struct cpc_endpoint *ep);
+};
+
+int __cpc_driver_register(struct cpc_driver *cpc_drv, struct module *owner);
+void cpc_driver_unregister(struct cpc_driver *cpc_drv);
+
+/* Convenience macro with THIS_MODULE */
+#define cpc_driver_register(driver) \
+ __cpc_driver_register(driver, THIS_MODULE)
+
+/**
+ * cpc_driver_from_drv - Upcast from a device driver.
+ * @drv: Reference to a device driver.
+ *
+ * @return: Reference to the cpc driver.
+ */
+static inline struct cpc_driver *cpc_driver_from_drv(const struct device_driver *drv)
+{
+ return container_of(drv, struct cpc_driver, driver);
+}
+
#endif
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
index 5aef8d7e43c..98e49614320 100644
--- a/drivers/net/cpc/endpoint.c
+++ b/drivers/net/cpc/endpoint.c
@@ -48,6 +48,7 @@ struct cpc_endpoint *cpc_endpoint_alloc(struct cpc_interface *intf, u8 id)
ep->id = id;
ep->dev.parent = &intf->dev;
+ ep->dev.bus = &cpc_bus;
ep->dev.release = cpc_ep_release;
device_initialize(&ep->dev);
diff --git a/drivers/net/cpc/main.c b/drivers/net/cpc/main.c
index ba9ab1ccf63..dcbe6dcb651 100644
--- a/drivers/net/cpc/main.c
+++ b/drivers/net/cpc/main.c
@@ -3,16 +3,79 @@
* Copyright (c) 2025, Silicon Laboratories, Inc.
*/
+#include <linux/device/driver.h>
#include <linux/module.h>
+#include "cpc.h"
+
+static int cpc_bus_match(struct device *dev, const struct device_driver *driver)
+{
+ struct cpc_driver *cpc_drv = cpc_driver_from_drv(driver);
+ struct cpc_endpoint *cpc_ep = cpc_endpoint_from_dev(dev);
+
+ return strcmp(cpc_drv->driver.name, cpc_ep->name) == 0;
+}
+
+static int cpc_bus_probe(struct device *dev)
+{
+ struct cpc_driver *cpc_drv = cpc_driver_from_drv(dev->driver);
+ struct cpc_endpoint *ep = cpc_endpoint_from_dev(dev);
+
+ return cpc_drv->probe(ep);
+}
+
+static void cpc_bus_remove(struct device *dev)
+{
+ struct cpc_driver *cpc_drv = cpc_driver_from_drv(dev->driver);
+ struct cpc_endpoint *ep = cpc_endpoint_from_dev(dev);
+
+ cpc_drv->remove(ep);
+}
+
+const struct bus_type cpc_bus = {
+ .name = KBUILD_MODNAME,
+ .match = cpc_bus_match,
+ .probe = cpc_bus_probe,
+ .remove = cpc_bus_remove,
+};
+
+/**
+ * __cpc_driver_register() - Register driver to the cpc bus.
+ * @cpc_drv: Reference to the cpc driver.
+ * @owner: Reference to this module's owner.
+ *
+ * @return: 0 on success, otherwise a negative error code.
+ */
+int __cpc_driver_register(struct cpc_driver *cpc_drv, struct module *owner)
+{
+ cpc_drv->driver.bus = &cpc_bus;
+ cpc_drv->driver.owner = owner;
+
+ return driver_register(&cpc_drv->driver);
+}
+
+/**
+ * cpc_driver_unregister() - Unregister driver from the cpc bus.
+ * @cpc_drv: Reference to the cpc driver.
+ */
+void cpc_driver_unregister(struct cpc_driver *cpc_drv)
+{
+ driver_unregister(&cpc_drv->driver);
+}
+
static int __init cpc_init(void)
{
- return 0;
+ int err;
+
+ err = bus_register(&cpc_bus);
+
+ return err;
}
module_init(cpc_init);
static void __exit cpc_exit(void)
{
+ bus_unregister(&cpc_bus);
}
module_exit(cpc_exit);
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 04/15] net: cpc: add protocol header structure and API
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (2 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 03/15] net: cpc: introduce CPC driver and bus Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 2:41 ` Andrew Lunn
2025-05-12 1:27 ` [RFC net-next 05/15] net: cpc: implement basic transmit path Damien Riégel
` (11 subsequent siblings)
15 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
CPC frames are composed of a header and optionally a payload. There are
three main frame types currently supported:
- DATA for transmitting payload or regular control frames (like ACKs,
that don't have payload associated with them)
- SYN for connection sequence
- RST for disconnection and errors
Add structure and functions to operate on this header. They will be
leveraged in a future commit where the protocol is actually implemented.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/Makefile | 2 +-
| 237 +++++++++++++++++++++++++++++++++++++++
| 83 ++++++++++++++
3 files changed, 321 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/cpc/header.c
create mode 100644 drivers/net/cpc/header.h
diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
index 673a40db424..81c470012c1 100644
--- a/drivers/net/cpc/Makefile
+++ b/drivers/net/cpc/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-cpc-y := endpoint.o interface.o main.o
+cpc-y := endpoint.o header.o interface.o main.o
obj-$(CONFIG_CPC) += cpc.o
--git a/drivers/net/cpc/header.c b/drivers/net/cpc/header.c
new file mode 100644
index 00000000000..9f6d637b5ae
--- /dev/null
+++ b/drivers/net/cpc/header.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/string.h>
+
+#include "header.h"
+
+#define CPC_CONTROL_TYPE_MASK 0xC0
+#define CPC_CONTROL_ACK_MASK BIT(2)
+
+/**
+ * cpc_header_get_type() - Get the frame type.
+ * @hdr_raw: Raw header.
+ * @type: Reference to a frame type.
+ *
+ * Return: True if the type has been successfully decoded, otherwise false.
+ * On success, the output parameter type is assigned.
+ */
+bool cpc_header_get_type(const u8 hdr_raw[CPC_HEADER_SIZE], enum cpc_frame_type *type)
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ switch (FIELD_GET(CPC_CONTROL_TYPE_MASK, hdr->ctrl)) {
+ case CPC_FRAME_TYPE_DATA:
+ *type = CPC_FRAME_TYPE_DATA;
+ break;
+ case CPC_FRAME_TYPE_SYN:
+ *type = CPC_FRAME_TYPE_SYN;
+ break;
+ case CPC_FRAME_TYPE_RST:
+ *type = CPC_FRAME_TYPE_RST;
+ break;
+ default:
+ return false;
+ }
+
+ return true;
+}
+
+/**
+ * cpc_header_get_ep_id() - Get the endpoint id.
+ * @hdr_raw: Raw header.
+ *
+ * Return: Endpoint id.
+ */
+u8 cpc_header_get_ep_id(const u8 hdr_raw[CPC_HEADER_SIZE])
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ return hdr->ep_id;
+}
+
+/**
+ * cpc_header_get_recv_wnd() - Get the receive window.
+ * @hdr_raw: Raw header.
+ *
+ * Return: Receive window.
+ */
+u8 cpc_header_get_recv_wnd(const u8 hdr_raw[CPC_HEADER_SIZE])
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ return hdr->recv_wnd;
+}
+
+/**
+ * cpc_header_get_seq() - Get the sequence number.
+ * @hdr_raw: Raw header.
+ *
+ * Return: Sequence number.
+ */
+u8 cpc_header_get_seq(const u8 hdr_raw[CPC_HEADER_SIZE])
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ return hdr->seq;
+}
+
+/**
+ * cpc_header_get_ack() - Get the acknowledge number.
+ * @hdr_raw: Raw header.
+ *
+ * Return: Acknowledge number.
+ */
+u8 cpc_header_get_ack(const u8 hdr_raw[CPC_HEADER_SIZE])
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ return hdr->ack;
+}
+
+/**
+ * cpc_header_get_req_ack() - Get the request acknowledge frame flag.
+ * @hdr_raw: Raw header.
+ *
+ * Return: Request acknowledge frame flag.
+ */
+bool cpc_header_get_req_ack(const u8 hdr_raw[CPC_HEADER_SIZE])
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ return FIELD_GET(CPC_CONTROL_ACK_MASK, hdr->ctrl);
+}
+
+/**
+ * cpc_header_get_mtu() - Get the maximum transmission unit.
+ * @hdr_raw: Raw header.
+ *
+ * Return: Maximum transmission unit.
+ *
+ * Must only be used over a SYN frame.
+ */
+u16 cpc_header_get_mtu(const u8 hdr_raw[CPC_HEADER_SIZE])
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ return le16_to_cpu(hdr->syn.mtu);
+}
+
+/**
+ * cpc_header_get_payload_len() - Get the payload length.
+ * @hdr_raw: Raw header.
+ *
+ * Return: Payload length.
+ *
+ * Must only be used over a DATA frame.
+ */
+u16 cpc_header_get_payload_len(const u8 hdr_raw[CPC_HEADER_SIZE])
+{
+ const struct cpc_header *hdr = (struct cpc_header *)hdr_raw;
+
+ return le16_to_cpu(hdr->dat.payload_len);
+}
+
+/**
+ * cpc_header_get_ctrl() - Encode parameters into a control byte.
+ * @type: Frame type.
+ * @req_ack: Frame flag indicating a request to be acknowledged.
+ *
+ * Return: Encoded control byte.
+ */
+u8 cpc_header_get_ctrl(enum cpc_frame_type type, bool req_ack)
+{
+ return FIELD_PREP(CPC_CONTROL_TYPE_MASK, type) |
+ FIELD_PREP(CPC_CONTROL_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.
+ * @ack_pending_count: Amount of frames pending on an acknowledge.
+ *
+ * Return: Frames to be acknowledged.
+ */
+u8 cpc_header_get_frames_acked_count(u8 seq, u8 ack, u8 ack_pending_count)
+{
+ u8 frames_acked_count;
+ u8 ack_range_min;
+ u8 ack_range_max;
+
+ ack_range_min = seq + 1;
+ ack_range_max = seq + ack_pending_count;
+
+ if (!cpc_header_number_in_range(ack_range_min, ack_range_max, ack))
+ return 0;
+
+ /* Find number of frames acknowledged with ACK number. */
+ if (ack > seq) {
+ frames_acked_count = ack - seq;
+ } else {
+ frames_acked_count = 256 - seq;
+ frames_acked_count += ack;
+ }
+
+ return frames_acked_count;
+}
+
+/**
+ * cpc_header_is_syn_ack_valid() - Check if the provided SYN-ACK valid or not.
+ * @seq: Current sequence number of the endpoint.
+ * @ack: Acknowledge number of the received SYN.
+ *
+ * Return: True if valid, otherwise false.
+ */
+bool cpc_header_is_syn_ack_valid(u8 seq, u8 ack)
+{
+ return !!cpc_header_get_frames_acked_count(seq, ack, 1);
+}
+
+/**
+ * cpc_header_number_in_window() - Test if a number is within a window.
+ * @start: Start of the window.
+ * @end: 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;
+}
--git a/drivers/net/cpc/header.h b/drivers/net/cpc/header.h
new file mode 100644
index 00000000000..c85bcaef345
--- /dev/null
+++ b/drivers/net/cpc/header.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_HEADER_H
+#define __CPC_HEADER_H
+
+#include <linux/compiler_attributes.h>
+#include <linux/types.h>
+
+#define CPC_HEADER_MAX_RX_WINDOW 255
+#define CPC_HEADER_SIZE 8
+
+/**
+ * enum cpc_frame_type - Describes all possible frame types that can
+ * be received or sent.
+ * @CPC_FRAME_TYPE_DATA: Used to send and control application DATA frames.
+ * @CPC_FRAME_TYPE_SYN: Used to initiate an endpoint connection.
+ * @CPC_FRAME_TYPE_RST: Used to reset the endpoint connection and indicate
+ * that the endpoint is unavailable.
+ */
+enum cpc_frame_type {
+ CPC_FRAME_TYPE_DATA,
+ CPC_FRAME_TYPE_SYN,
+ CPC_FRAME_TYPE_RST,
+};
+
+/**
+ * struct cpc_header - Representation of the CPC header.
+ * @ctrl: Indicates the frame type [7..6] and frame flags [5..0].
+ * Currently only the request acknowledge flag is supported.
+ * This flag indicates if the frame should be acknowledged by
+ * the remote on reception.
+ * @ep_id: Address of the endpoint the frame is destined to.
+ * @recv_wnd: Indicates to the remote how many reception buffers are
+ * available so it can determine how many frames it can send.
+ * @seq: Identifies the frame with a number.
+ * @ack: Indicate the sequence number of the next expected frame from
+ * the remote. When paired with a fast re-transmit flag, it indicates
+ * the sequence number of the frame in error that should be
+ * re-transmitted.
+ * @syn.mtu: On a SYN frame, this represents the maximum transmission unit.
+ * @dat.payload_len: On a DATA frame, this indicates the payload length.
+ */
+struct cpc_header {
+ u8 ctrl;
+ u8 ep_id;
+ u8 recv_wnd;
+ u8 seq;
+ u8 ack;
+ union {
+ u8 extension[3];
+ struct __packed {
+ __le16 mtu;
+ u8 reserved;
+ } syn;
+ struct __packed {
+ __le16 payload_len;
+ u8 reserved;
+ } dat;
+ struct __packed {
+ u8 reserved[3];
+ } rst;
+ };
+} __packed;
+
+bool cpc_header_get_type(const u8 hdr_raw[CPC_HEADER_SIZE], enum cpc_frame_type *type);
+u8 cpc_header_get_ep_id(const u8 hdr_raw[CPC_HEADER_SIZE]);
+u8 cpc_header_get_recv_wnd(const u8 hdr_raw[CPC_HEADER_SIZE]);
+u8 cpc_header_get_seq(const u8 hdr_raw[CPC_HEADER_SIZE]);
+u8 cpc_header_get_ack(const u8 hdr_raw[CPC_HEADER_SIZE]);
+bool cpc_header_get_req_ack(const u8 hdr_raw[CPC_HEADER_SIZE]);
+u16 cpc_header_get_mtu(const u8 hdr_raw[CPC_HEADER_SIZE]);
+u16 cpc_header_get_payload_len(const u8 hdr_raw[CPC_HEADER_SIZE]);
+u8 cpc_header_get_ctrl(enum cpc_frame_type type, bool req_ack);
+
+u8 cpc_header_get_frames_acked_count(u8 seq, u8 ack, u8 ack_pending_count);
+bool cpc_header_is_syn_ack_valid(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
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC net-next 04/15] net: cpc: add protocol header structure and API
2025-05-12 1:27 ` [RFC net-next 04/15] net: cpc: add protocol header structure and API Damien Riégel
@ 2025-05-12 2:41 ` Andrew Lunn
0 siblings, 0 replies; 39+ messages in thread
From: Andrew Lunn @ 2025-05-12 2:41 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
> +struct cpc_header {
> + u8 ctrl;
> + u8 ep_id;
> + u8 recv_wnd;
> + u8 seq;
> + u8 ack;
> + union {
> + u8 extension[3];
> + struct __packed {
> + __le16 mtu;
> + u8 reserved;
> + } syn;
> + struct __packed {
> + __le16 payload_len;
> + u8 reserved;
These two le16 are unaligned for no good reason. Put the reserved byte
first, then the u16. Once you have done that, you might be able to
throw away all the __packed because it is then all naturally aligned.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC net-next 05/15] net: cpc: implement basic transmit path
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (3 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 04/15] net: cpc: add protocol header structure and API Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 06/15] net: cpc: implement basic receive path Damien Riégel
` (10 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Implement a very basic transmission path for endpoints. Transmitting
involves the following flow:
- user call cpc_endpoint_write() with the SKB they wish to send
- a CPC header is prepared and prepended to the frame
- SKB is pushed to endpoint's holding queue (*)
- SKB is dequeued from endpoint's holding queue and moved to
interface's transmit queue (*)
- interface is woken up by invoking its wake_tx() callback
- interface work task is now responsible for dequeueing and
transmitting that SKB when it's convenient for it to do so.
(*) Endpoint's holding queue currently serves no purpose but it will
when other protocol features are implemented.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/Makefile | 2 +-
drivers/net/cpc/cpc.h | 15 +++++++++
drivers/net/cpc/endpoint.c | 33 +++++++++++++++++++
drivers/net/cpc/interface.c | 40 +++++++++++++++++++++++
drivers/net/cpc/interface.h | 7 +++++
drivers/net/cpc/main.c | 50 +++++++++++++++++++++++++++++
drivers/net/cpc/protocol.c | 63 +++++++++++++++++++++++++++++++++++++
drivers/net/cpc/protocol.h | 19 +++++++++++
8 files changed, 228 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/cpc/protocol.c
create mode 100644 drivers/net/cpc/protocol.h
diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
index 81c470012c1..0e9c3f775dc 100644
--- a/drivers/net/cpc/Makefile
+++ b/drivers/net/cpc/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-cpc-y := endpoint.o header.o interface.o main.o
+cpc-y := endpoint.o header.o interface.o main.o protocol.o
obj-$(CONFIG_CPC) += cpc.o
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
index cbd1b3d6a03..2f54e5b660e 100644
--- a/drivers/net/cpc/cpc.h
+++ b/drivers/net/cpc/cpc.h
@@ -7,6 +7,7 @@
#define __CPC_H
#include <linux/device.h>
+#include <linux/skbuff.h>
#include <linux/types.h>
#define CPC_ENDPOINT_NAME_MAX_LEN 128
@@ -24,6 +25,8 @@ extern const struct bus_type cpc_bus;
* @id: Endpoint id, uniquely identifies an endpoint within a CPC device.
* @intf: Pointer to CPC device this endpoint belongs to.
* @list_node: list_head member for linking in a CPC device.
+ * @holding_queue: Contains frames that were not pushed to the transport layer
+ * due to having insufficient space in the transmit window.
*
* Each endpoint can send and receive data without consideration of the other endpoints sharing the
* same physical link.
@@ -36,6 +39,8 @@ struct cpc_endpoint {
struct cpc_interface *intf;
struct list_head list_node;
+
+ struct sk_buff_head holding_queue;
};
struct cpc_endpoint *cpc_endpoint_alloc(struct cpc_interface *intf, u8 id);
@@ -44,6 +49,8 @@ struct cpc_endpoint *cpc_endpoint_new(struct cpc_interface *intf, u8 id, const c
void cpc_endpoint_unregister(struct cpc_endpoint *ep);
+int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb);
+
/**
* cpc_endpoint_from_dev() - Upcast from a device pointer.
* @dev: Reference to a device.
@@ -137,4 +144,12 @@ static inline struct cpc_driver *cpc_driver_from_drv(const struct device_driver
return container_of(drv, struct cpc_driver, driver);
}
+/*---------------------------------------------------------------------------*/
+
+struct sk_buff *cpc_skb_alloc(size_t payload_len, gfp_t priority);
+void cpc_skb_set_ctx(struct sk_buff *skb,
+ void (*destructor)(struct sk_buff *skb),
+ void *ctx);
+void *cpc_skb_get_ctx(struct sk_buff *skb);
+
#endif
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
index 98e49614320..4e98955be30 100644
--- a/drivers/net/cpc/endpoint.c
+++ b/drivers/net/cpc/endpoint.c
@@ -6,7 +6,9 @@
#include <linux/string.h>
#include "cpc.h"
+#include "header.h"
#include "interface.h"
+#include "protocol.h"
/**
* cpc_ep_release() - Actual release of the CPC endpoint.
@@ -18,6 +20,8 @@ static void cpc_ep_release(struct device *dev)
{
struct cpc_endpoint *ep = cpc_endpoint_from_dev(dev);
+ skb_queue_purge(&ep->holding_queue);
+
cpc_interface_put(ep->intf);
kfree(ep);
}
@@ -51,6 +55,8 @@ struct cpc_endpoint *cpc_endpoint_alloc(struct cpc_interface *intf, u8 id)
ep->dev.bus = &cpc_bus;
ep->dev.release = cpc_ep_release;
+ skb_queue_head_init(&ep->holding_queue);
+
device_initialize(&ep->dev);
return ep;
@@ -165,3 +171,30 @@ void cpc_endpoint_unregister(struct cpc_endpoint *ep)
device_del(&ep->dev);
put_device(&ep->dev);
}
+
+/**
+ * cpc_endpoint_write - Write a DATA frame.
+ * @ep: Endpoint handle.
+ * @skb: Frame to send.
+ *
+ * @return: 0 on success, otherwise a negative error code.
+ */
+int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ struct cpc_header hdr;
+ int err;
+
+ if (ep->intf->ops->csum)
+ ep->intf->ops->csum(skb);
+
+ memset(&hdr, 0, sizeof(hdr));
+ hdr.ctrl = cpc_header_get_ctrl(CPC_FRAME_TYPE_DATA, true);
+ hdr.ep_id = ep->id;
+ hdr.recv_wnd = CPC_HEADER_MAX_RX_WINDOW;
+ hdr.seq = 0;
+ hdr.dat.payload_len = skb->len;
+
+ err = __cpc_protocol_write(ep, &hdr, skb);
+
+ return err;
+}
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
index 6b3fc16f212..1dd87deed59 100644
--- a/drivers/net/cpc/interface.c
+++ b/drivers/net/cpc/interface.c
@@ -58,6 +58,8 @@ struct cpc_interface *cpc_interface_alloc(struct device *parent,
mutex_init(&intf->lock);
INIT_LIST_HEAD(&intf->eps);
+ skb_queue_head_init(&intf->tx_queue);
+
intf->ops = ops;
intf->dev.parent = parent;
@@ -154,3 +156,41 @@ struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 e
return ep;
}
+
+/**
+ * cpc_interface_send_frame() - Queue a socket buffer for transmission.
+ * @intf: Interface to send SKB over.
+ * @ops: SKB to send.
+ *
+ * Queue SKB in interface's transmit queue and signal the interface. Interface is expected to use
+ * cpc_interface_dequeue() to get the next SKB to transmit.
+ */
+void cpc_interface_send_frame(struct cpc_interface *intf, struct sk_buff *skb)
+{
+ skb_queue_tail(&intf->tx_queue, skb);
+ intf->ops->wake_tx(intf);
+}
+
+/**
+ * cpc_interface_dequeue() - Get the next SKB that was queued for transmission.
+ * @intf: Interface.
+ *
+ * Get an SKB that was previously queued by cpc_interface_send_frame().
+ *
+ * Return: An SKB, or %NULL if queue was empty.
+ */
+struct sk_buff *cpc_interface_dequeue(struct cpc_interface *intf)
+{
+ return skb_dequeue(&intf->tx_queue);
+}
+
+/**
+ * cpc_interface_tx_queue_empty() - Check if transmit queue is empty.
+ * @intf: Interface.
+ *
+ * Return: True if transmit queue is empty, false otherwise.
+ */
+bool cpc_interface_tx_queue_empty(struct cpc_interface *intf)
+{
+ return skb_queue_empty_lockless(&intf->tx_queue);
+}
diff --git a/drivers/net/cpc/interface.h b/drivers/net/cpc/interface.h
index d6b6d9ce5de..1b501b1f6dc 100644
--- a/drivers/net/cpc/interface.h
+++ b/drivers/net/cpc/interface.h
@@ -22,6 +22,7 @@ struct cpc_interface_ops;
* @index: Device index.
* @lock: Protect access to endpoint list.
* @eps: List of endpoints managed by this device.
+ * @tx_queue: Transmit queue to be consumed by the interface.
*/
struct cpc_interface {
struct device dev;
@@ -35,6 +36,8 @@ struct cpc_interface {
struct mutex lock; /* Protect eps from concurrent access. */
struct list_head eps;
+
+ struct sk_buff_head tx_queue;
};
/**
@@ -58,6 +61,10 @@ void cpc_interface_unregister(struct cpc_interface *intf);
struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id);
+void cpc_interface_send_frame(struct cpc_interface *intf, struct sk_buff *skb);
+struct sk_buff *cpc_interface_dequeue(struct cpc_interface *intf);
+bool cpc_interface_tx_queue_empty(struct cpc_interface *intf);
+
/**
* cpc_interface_get() - Get a reference to interface and return its pointer.
* @intf: Interface to get.
diff --git a/drivers/net/cpc/main.c b/drivers/net/cpc/main.c
index dcbe6dcb651..8feb0613252 100644
--- a/drivers/net/cpc/main.c
+++ b/drivers/net/cpc/main.c
@@ -7,6 +7,56 @@
#include <linux/module.h>
#include "cpc.h"
+#include "header.h"
+
+/**
+ * cpc_skb_alloc() - Allocate an skb with a specific headroom for CPC headers.
+ * @payload_len: Length of the payload.
+ * @priority: GFP priority to use for memory allocation.
+ *
+ * Return: Pointer to the skb on success, otherwise NULL.
+ */
+struct sk_buff *cpc_skb_alloc(size_t payload_len, gfp_t priority)
+{
+ struct sk_buff *skb;
+
+ skb = alloc_skb(payload_len + CPC_HEADER_SIZE, priority);
+ if (skb)
+ skb_reserve(skb, CPC_HEADER_SIZE);
+
+ return skb;
+}
+
+/**
+ * cpc_skb_set_ctx() - Set the skb context.
+ * @skb: Frame.
+ * @destructor: Destructor callback.
+ * @ctx: Context pointer, might be NULL.
+ */
+void cpc_skb_set_ctx(struct sk_buff *skb,
+ void (*destructor)(struct sk_buff *skb),
+ void *ctx)
+{
+ skb->destructor = destructor;
+
+ if (ctx)
+ memcpy(&skb->cb[0], &ctx, sizeof(void *));
+}
+
+/**
+ * cpc_skb_get_ctx() - Get the skb context.
+ * @skb: Frame.
+ *
+ * Return: Context pointer.
+ */
+void *cpc_skb_get_ctx(struct sk_buff *skb)
+{
+ void *ctx;
+
+ memcpy(&ctx, &skb->cb[0], sizeof(void *));
+
+ return ctx;
+}
static int cpc_bus_match(struct device *dev, const struct device_driver *driver)
{
diff --git a/drivers/net/cpc/protocol.c b/drivers/net/cpc/protocol.c
new file mode 100644
index 00000000000..692d3e07939
--- /dev/null
+++ b/drivers/net/cpc/protocol.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/mutex.h>
+#include <linux/skbuff.h>
+
+#include "cpc.h"
+#include "header.h"
+#include "interface.h"
+#include "protocol.h"
+
+static int __cpc_protocol_queue_tx_frame(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ struct cpc_interface *intf = ep->intf;
+ struct sk_buff *cloned_skb;
+
+ cloned_skb = skb_clone(skb, GFP_KERNEL);
+ if (!cloned_skb)
+ return -ENOMEM;
+
+ cpc_interface_send_frame(intf, cloned_skb);
+
+ return 0;
+}
+
+static void __cpc_protocol_process_pending_tx_frames(struct cpc_endpoint *ep)
+{
+ struct sk_buff *skb;
+ int err;
+
+ while ((skb = skb_dequeue(&ep->holding_queue))) {
+ err = __cpc_protocol_queue_tx_frame(ep, skb);
+ if (err < 0) {
+ skb_queue_head(&ep->holding_queue, skb);
+ return;
+ }
+ }
+}
+
+/**
+ * __cpc_protocol_write() - Write a frame.
+ * @ep: Endpoint handle.
+ * @hdr: Header to write.
+ * @skb: Payload to write.
+ *
+ * Context: Expect endpoint's lock to be held.
+ *
+ * Return: 0 on success, otherwise a negative error code.
+ */
+int __cpc_protocol_write(struct cpc_endpoint *ep,
+ struct cpc_header *hdr,
+ struct sk_buff *skb)
+{
+ memcpy(skb_push(skb, sizeof(*hdr)), hdr, sizeof(*hdr));
+
+ skb_queue_tail(&ep->holding_queue, skb);
+
+ __cpc_protocol_process_pending_tx_frames(ep);
+
+ return 0;
+}
diff --git a/drivers/net/cpc/protocol.h b/drivers/net/cpc/protocol.h
new file mode 100644
index 00000000000..b51f0191be4
--- /dev/null
+++ b/drivers/net/cpc/protocol.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef CPC_PROTOCOL_H
+#define CPC_PROTOCOL_H
+
+#include <linux/skbuff.h>
+#include <linux/timer.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+struct cpc_endpoint;
+struct cpc_header;
+
+int __cpc_protocol_write(struct cpc_endpoint *ep, struct cpc_header *hdr, struct sk_buff *skb);
+
+#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 06/15] net: cpc: implement basic receive path
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (4 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 05/15] net: cpc: implement basic transmit path Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 07/15] net: cpc: implement sequencing and ack Damien Riégel
` (9 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Implement a very basic receive path. When a new frame is available,
interfaces are expected to call cpc_interface_receive_frame(). This
frame will be handled in a high-priority workqueue and dispatched to the
endpoint it targets, if available.
Endpoints should set an RX callback with cpc_endpoint_set_ops() in order
to be notified when a new frame arrives. This callback should be short,
long operations should be dispatched or the main reception task will be
blocked until that processing finishes.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/cpc.h | 9 ++++++
drivers/net/cpc/endpoint.c | 11 +++++++
drivers/net/cpc/interface.c | 59 +++++++++++++++++++++++++++++++++++++
drivers/net/cpc/interface.h | 8 +++++
drivers/net/cpc/protocol.c | 15 ++++++++++
drivers/net/cpc/protocol.h | 2 ++
6 files changed, 104 insertions(+)
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
index 2f54e5b660e..dc05b36b6e6 100644
--- a/drivers/net/cpc/cpc.h
+++ b/drivers/net/cpc/cpc.h
@@ -18,6 +18,13 @@ struct cpc_endpoint;
extern const struct bus_type cpc_bus;
+/** struct cpc_endpoint_ops - Endpoint's callbacks.
+ * @rx: Data availability is provided with a skb owned by the driver.
+ */
+struct cpc_endpoint_ops {
+ void (*rx)(struct cpc_endpoint *ep, struct sk_buff *skb);
+};
+
/**
* struct cpc_endpoint - Representation of CPC endpointl
* @dev: Driver model representation of the device.
@@ -39,6 +46,7 @@ struct cpc_endpoint {
struct cpc_interface *intf;
struct list_head list_node;
+ struct cpc_endpoint_ops *ops;
struct sk_buff_head holding_queue;
};
@@ -50,6 +58,7 @@ struct cpc_endpoint *cpc_endpoint_new(struct cpc_interface *intf, u8 id, const c
void cpc_endpoint_unregister(struct cpc_endpoint *ep);
int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb);
+void cpc_endpoint_set_ops(struct cpc_endpoint *ep, struct cpc_endpoint_ops *ops);
/**
* cpc_endpoint_from_dev() - Upcast from a device pointer.
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
index 4e98955be30..51007ba5bcc 100644
--- a/drivers/net/cpc/endpoint.c
+++ b/drivers/net/cpc/endpoint.c
@@ -172,6 +172,17 @@ void cpc_endpoint_unregister(struct cpc_endpoint *ep)
put_device(&ep->dev);
}
+/**
+ * cpc_endpoint_set_ops() - Set callbacks for this endpoint.
+ * @ep: Endpoint
+ * @ops: New callbacks to set. If already set, override pre-existing value.
+ */
+void cpc_endpoint_set_ops(struct cpc_endpoint *ep, struct cpc_endpoint_ops *ops)
+{
+ if (ep)
+ ep->ops = ops;
+}
+
/**
* cpc_endpoint_write - Write a DATA frame.
* @ep: Endpoint handle.
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
index 1dd87deed59..edc6b387e50 100644
--- a/drivers/net/cpc/interface.c
+++ b/drivers/net/cpc/interface.c
@@ -6,12 +6,44 @@
#include <linux/module.h>
#include "cpc.h"
+#include "header.h"
#include "interface.h"
+#include "protocol.h"
#define to_cpc_interface(d) container_of(d, struct cpc_interface, dev)
static DEFINE_IDA(cpc_ida);
+static void cpc_interface_rx_work(struct work_struct *work)
+{
+ struct cpc_interface *intf = container_of(work, struct cpc_interface, rx_work);
+ enum cpc_frame_type type;
+ struct cpc_endpoint *ep;
+ struct sk_buff *skb;
+ u8 ep_id;
+
+ while ((skb = skb_dequeue(&intf->rx_queue))) {
+ cpc_header_get_type(skb->data, &type);
+ ep_id = cpc_header_get_ep_id(skb->data);
+
+ ep = cpc_interface_get_endpoint(intf, ep_id);
+ if (!ep) {
+ kfree_skb(skb);
+ continue;
+ }
+
+ switch (type) {
+ case CPC_FRAME_TYPE_DATA:
+ cpc_protocol_on_data(ep, skb);
+ break;
+ default:
+ kfree_skb(skb);
+ }
+
+ cpc_endpoint_put(ep);
+ }
+}
+
/**
* cpc_intf_release() - Actual release of interface.
* @dev: Device embedded in struct cpc_interface
@@ -23,6 +55,10 @@ static void cpc_intf_release(struct device *dev)
{
struct cpc_interface *intf = to_cpc_interface(dev);
+ flush_work(&intf->rx_work);
+
+ destroy_workqueue(intf->workq);
+
ida_free(&cpc_ida, intf->index);
kfree(intf);
}
@@ -54,10 +90,20 @@ struct cpc_interface *cpc_interface_alloc(struct device *parent,
return NULL;
}
+ intf->workq = alloc_workqueue(KBUILD_MODNAME "_wq", WQ_HIGHPRI, 0);
+ if (!intf->workq) {
+ ida_free(&cpc_ida, intf->index);
+ kfree(intf);
+
+ return ERR_PTR(-ENOMEM);
+ }
+
mutex_init(&intf->add_lock);
mutex_init(&intf->lock);
INIT_LIST_HEAD(&intf->eps);
+ INIT_WORK(&intf->rx_work, cpc_interface_rx_work);
+ skb_queue_head_init(&intf->rx_queue);
skb_queue_head_init(&intf->tx_queue);
intf->ops = ops;
@@ -157,6 +203,19 @@ struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 e
return ep;
}
+/**
+ * cpc_interface_receive_frame - queue a received frame for processing
+ * @intf: pointer to the CPC device
+ * @skb: received frame
+ *
+ * Context: This queues the sk_buff in a list and schedule the work task to process the list.
+ */
+void cpc_interface_receive_frame(struct cpc_interface *intf, struct sk_buff *skb)
+{
+ skb_queue_tail(&intf->rx_queue, skb);
+ queue_work(intf->workq, &intf->rx_work);
+}
+
/**
* cpc_interface_send_frame() - Queue a socket buffer for transmission.
* @intf: Interface to send SKB over.
diff --git a/drivers/net/cpc/interface.h b/drivers/net/cpc/interface.h
index 1b501b1f6dc..a45227a50a7 100644
--- a/drivers/net/cpc/interface.h
+++ b/drivers/net/cpc/interface.h
@@ -22,6 +22,9 @@ struct cpc_interface_ops;
* @index: Device index.
* @lock: Protect access to endpoint list.
* @eps: List of endpoints managed by this device.
+ * @workq: Interface-specific work queue.
+ * @rx_work: work struct for processing received frames
+ * @rx_queue: list of sk_buff that were received
* @tx_queue: Transmit queue to be consumed by the interface.
*/
struct cpc_interface {
@@ -37,6 +40,10 @@ struct cpc_interface {
struct mutex lock; /* Protect eps from concurrent access. */
struct list_head eps;
+ struct workqueue_struct *workq;
+ struct work_struct rx_work;
+ struct sk_buff_head rx_queue;
+
struct sk_buff_head tx_queue;
};
@@ -61,6 +68,7 @@ void cpc_interface_unregister(struct cpc_interface *intf);
struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id);
+void cpc_interface_receive_frame(struct cpc_interface *intf, struct sk_buff *skb);
void cpc_interface_send_frame(struct cpc_interface *intf, struct sk_buff *skb);
struct sk_buff *cpc_interface_dequeue(struct cpc_interface *intf);
bool cpc_interface_tx_queue_empty(struct cpc_interface *intf);
diff --git a/drivers/net/cpc/protocol.c b/drivers/net/cpc/protocol.c
index 692d3e07939..91335160981 100644
--- a/drivers/net/cpc/protocol.c
+++ b/drivers/net/cpc/protocol.c
@@ -39,6 +39,21 @@ static void __cpc_protocol_process_pending_tx_frames(struct cpc_endpoint *ep)
}
}
+void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ if (skb->len > CPC_HEADER_SIZE) {
+ /* Strip header. */
+ skb_pull(skb, CPC_HEADER_SIZE);
+
+ if (ep->ops && ep->ops->rx)
+ ep->ops->rx(ep, skb);
+ else
+ kfree_skb(skb);
+ } else {
+ kfree_skb(skb);
+ }
+}
+
/**
* __cpc_protocol_write() - Write a frame.
* @ep: Endpoint handle.
diff --git a/drivers/net/cpc/protocol.h b/drivers/net/cpc/protocol.h
index b51f0191be4..9a028e0e94b 100644
--- a/drivers/net/cpc/protocol.h
+++ b/drivers/net/cpc/protocol.h
@@ -16,4 +16,6 @@ struct cpc_header;
int __cpc_protocol_write(struct cpc_endpoint *ep, struct cpc_header *hdr, struct sk_buff *skb);
+void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb);
+
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 07/15] net: cpc: implement sequencing and ack
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (5 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 06/15] net: cpc: implement basic receive path Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 08/15] net: cpc: add support for connecting endpoints Damien Riégel
` (8 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
CPC frames are sequenced and must be acked by the remote. If not acked
in a timely manner, they should be retransmitted but that feature is not
part of this commit.
Another key feature is that peers advertise how many frames they can
receive. As the remote is usually a microcontroller with limited memory,
this serves as a way to throttle the host and prevent it from sending
frames that the microcontroller is not yet able to receive. This is
where endpoint's holding_queue becomes useful and serves as storage for
frames that endpoint is ready to send but that the remote is not yet
able to receive.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/cpc.h | 26 +++++++++
drivers/net/cpc/endpoint.c | 24 ++++++++-
drivers/net/cpc/protocol.c | 108 ++++++++++++++++++++++++++++++++++++-
3 files changed, 156 insertions(+), 2 deletions(-)
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
index dc05b36b6e6..94284e2d59d 100644
--- a/drivers/net/cpc/cpc.h
+++ b/drivers/net/cpc/cpc.h
@@ -18,6 +18,27 @@ struct cpc_endpoint;
extern const struct bus_type cpc_bus;
+/**
+ * struct cpc_endpoint_tcb - endpoint's transmission control block
+ * @lock: synchronize tcb access
+ * @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
+ * @mtu: maximum transmission unit
+ */
+struct cpc_endpoint_tcb {
+ struct mutex lock; /* Synchronize access to all other attributes. */
+ u8 send_wnd;
+ u8 send_nxt;
+ u8 send_una;
+ u8 ack;
+ u8 seq;
+};
+
/** struct cpc_endpoint_ops - Endpoint's callbacks.
* @rx: Data availability is provided with a skb owned by the driver.
*/
@@ -32,6 +53,8 @@ struct cpc_endpoint_ops {
* @id: Endpoint id, uniquely identifies an endpoint within a CPC device.
* @intf: Pointer to CPC device this endpoint belongs to.
* @list_node: list_head member for linking in a CPC device.
+ * @tcb: Transmission control block.
+ * @pending_ack_queue: Contain frames pending on an acknowledge.
* @holding_queue: Contains frames that were not pushed to the transport layer
* due to having insufficient space in the transmit window.
*
@@ -48,6 +71,9 @@ struct cpc_endpoint {
struct list_head list_node;
struct cpc_endpoint_ops *ops;
+ struct cpc_endpoint_tcb tcb;
+
+ struct sk_buff_head pending_ack_queue;
struct sk_buff_head holding_queue;
};
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
index 51007ba5bcc..db925cc078d 100644
--- a/drivers/net/cpc/endpoint.c
+++ b/drivers/net/cpc/endpoint.c
@@ -20,12 +20,26 @@ static void cpc_ep_release(struct device *dev)
{
struct cpc_endpoint *ep = cpc_endpoint_from_dev(dev);
+ skb_queue_purge(&ep->pending_ack_queue);
skb_queue_purge(&ep->holding_queue);
cpc_interface_put(ep->intf);
kfree(ep);
}
+/**
+ * cpc_endpoint_tcb_reset() - Reset endpoint's TCB to initial values.
+ * @ep: endpoint pointer
+ */
+static void cpc_endpoint_tcb_reset(struct cpc_endpoint *ep)
+{
+ ep->tcb.seq = ep->id;
+ ep->tcb.ack = 0;
+ ep->tcb.send_nxt = ep->id;
+ ep->tcb.send_una = ep->id;
+ ep->tcb.send_wnd = 1;
+}
+
/**
* cpc_endpoint_alloc() - Allocate memory for new CPC endpoint.
* @intf: CPC interface owning this endpoint.
@@ -55,6 +69,10 @@ struct cpc_endpoint *cpc_endpoint_alloc(struct cpc_interface *intf, u8 id)
ep->dev.bus = &cpc_bus;
ep->dev.release = cpc_ep_release;
+ mutex_init(&ep->tcb.lock);
+ cpc_endpoint_tcb_reset(ep);
+
+ skb_queue_head_init(&ep->pending_ack_queue);
skb_queue_head_init(&ep->holding_queue);
device_initialize(&ep->dev);
@@ -195,6 +213,8 @@ int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb)
struct cpc_header hdr;
int err;
+ mutex_lock(&ep->tcb.lock);
+
if (ep->intf->ops->csum)
ep->intf->ops->csum(skb);
@@ -202,10 +222,12 @@ int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb)
hdr.ctrl = cpc_header_get_ctrl(CPC_FRAME_TYPE_DATA, true);
hdr.ep_id = ep->id;
hdr.recv_wnd = CPC_HEADER_MAX_RX_WINDOW;
- hdr.seq = 0;
+ hdr.seq = ep->tcb.seq;
hdr.dat.payload_len = skb->len;
err = __cpc_protocol_write(ep, &hdr, skb);
+ mutex_unlock(&ep->tcb.lock);
+
return err;
}
diff --git a/drivers/net/cpc/protocol.c b/drivers/net/cpc/protocol.c
index 91335160981..92e3b0a9cdf 100644
--- a/drivers/net/cpc/protocol.c
+++ b/drivers/net/cpc/protocol.c
@@ -11,15 +11,54 @@
#include "interface.h"
#include "protocol.h"
+static void __cpc_protocol_send_ack(struct cpc_endpoint *ep)
+{
+ struct cpc_header hdr;
+ struct sk_buff *skb;
+
+ skb = cpc_skb_alloc(0, GFP_KERNEL);
+ if (!skb)
+ return;
+
+ memset(&hdr, 0, sizeof(hdr));
+ hdr.ctrl = cpc_header_get_ctrl(CPC_FRAME_TYPE_DATA, false);
+ hdr.ep_id = ep->id;
+ hdr.recv_wnd = CPC_HEADER_MAX_RX_WINDOW;
+ hdr.ack = ep->tcb.ack;
+ memcpy(skb_push(skb, sizeof(hdr)), &hdr, sizeof(hdr));
+
+ cpc_interface_send_frame(ep->intf, skb);
+}
+
+static void cpc_protocol_on_tx_complete(struct sk_buff *skb)
+{
+ struct cpc_endpoint *ep = cpc_skb_get_ctx(skb);
+
+ /*
+ * Increase the send_nxt sequence, this is used as the upper bound of sequence number that
+ * can be ACK'd by the remote.
+ */
+ mutex_lock(&ep->tcb.lock);
+ ep->tcb.send_nxt++;
+ mutex_unlock(&ep->tcb.lock);
+}
+
static int __cpc_protocol_queue_tx_frame(struct cpc_endpoint *ep, struct sk_buff *skb)
{
+ struct cpc_header *hdr = (struct cpc_header *)skb->data;
struct cpc_interface *intf = ep->intf;
struct sk_buff *cloned_skb;
+ hdr->ack = ep->tcb.ack;
+
cloned_skb = skb_clone(skb, GFP_KERNEL);
if (!cloned_skb)
return -ENOMEM;
+ skb_queue_tail(&ep->pending_ack_queue, skb);
+
+ cpc_skb_set_ctx(cloned_skb, cpc_protocol_on_tx_complete, ep);
+
cpc_interface_send_frame(intf, cloned_skb);
return 0;
@@ -28,10 +67,19 @@ static int __cpc_protocol_queue_tx_frame(struct cpc_endpoint *ep, struct sk_buff
static void __cpc_protocol_process_pending_tx_frames(struct cpc_endpoint *ep)
{
struct sk_buff *skb;
+ u8 window;
int err;
+ window = ep->tcb.send_wnd;
+
while ((skb = skb_dequeue(&ep->holding_queue))) {
- err = __cpc_protocol_queue_tx_frame(ep, skb);
+ if (!cpc_header_number_in_window(ep->tcb.send_una,
+ window,
+ cpc_header_get_seq(skb->data)))
+ err = -ERANGE;
+ else
+ err = __cpc_protocol_queue_tx_frame(ep, skb);
+
if (err < 0) {
skb_queue_head(&ep->holding_queue, skb);
return;
@@ -39,8 +87,64 @@ static void __cpc_protocol_process_pending_tx_frames(struct cpc_endpoint *ep)
}
}
+static void __cpc_protocol_receive_ack(struct cpc_endpoint *ep, u8 recv_wnd, u8 ack)
+{
+ struct sk_buff *skb;
+ u8 acked_frames;
+
+ ep->tcb.send_wnd = recv_wnd;
+
+ skb = skb_peek(&ep->pending_ack_queue);
+ if (!skb)
+ goto out;
+
+ /* Return if no frame to ACK. */
+ if (!cpc_header_number_in_range(ep->tcb.send_una, ep->tcb.send_nxt, ack))
+ goto out;
+
+ /* Calculate how many frames will be ACK'd. */
+ acked_frames = cpc_header_get_frames_acked_count(cpc_header_get_seq(skb->data),
+ ack,
+ skb_queue_len(&ep->pending_ack_queue));
+
+ for (u8 i = 0; i < acked_frames; i++)
+ kfree_skb(skb_dequeue(&ep->pending_ack_queue));
+
+ ep->tcb.send_una += acked_frames;
+
+out:
+ __cpc_protocol_process_pending_tx_frames(ep);
+}
+
void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb)
{
+ bool expected_seq;
+
+ mutex_lock(&ep->tcb.lock);
+
+ __cpc_protocol_receive_ack(ep,
+ cpc_header_get_recv_wnd(skb->data),
+ cpc_header_get_ack(skb->data));
+
+ if (cpc_header_get_req_ack(skb->data)) {
+ expected_seq = cpc_header_get_seq(skb->data) == ep->tcb.ack;
+ if (expected_seq)
+ ep->tcb.ack++;
+
+ __cpc_protocol_send_ack(ep);
+
+ if (!expected_seq) {
+ dev_warn(&ep->dev,
+ "unexpected seq: %u, expected seq: %u\n",
+ cpc_header_get_seq(skb->data), ep->tcb.ack);
+ mutex_unlock(&ep->tcb.lock);
+ kfree_skb(skb);
+ return;
+ }
+ }
+
+ mutex_unlock(&ep->tcb.lock);
+
if (skb->len > CPC_HEADER_SIZE) {
/* Strip header. */
skb_pull(skb, CPC_HEADER_SIZE);
@@ -74,5 +178,7 @@ int __cpc_protocol_write(struct cpc_endpoint *ep,
__cpc_protocol_process_pending_tx_frames(ep);
+ ep->tcb.seq++;
+
return 0;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 08/15] net: cpc: add support for connecting endpoints
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (6 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 07/15] net: cpc: implement sequencing and ack Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 09/15] net: cpc: add support for RST frames Damien Riégel
` (7 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Endpoints must be connected before transmitting. This is achieved using
a three-way handshake in which each peer advertises the maximum payload
size it can receive. Once again, this constraint comes from the remote
microcontroller, which can have tight limit on the payload size it can
handle.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/cpc.h | 12 +++++
drivers/net/cpc/endpoint.c | 75 ++++++++++++++++++++++++++++
drivers/net/cpc/interface.c | 29 +++++++++++
drivers/net/cpc/interface.h | 3 ++
drivers/net/cpc/protocol.c | 97 ++++++++++++++++++++++++++++++++++++-
drivers/net/cpc/protocol.h | 4 ++
6 files changed, 219 insertions(+), 1 deletion(-)
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
index 94284e2d59d..d316fce4ad7 100644
--- a/drivers/net/cpc/cpc.h
+++ b/drivers/net/cpc/cpc.h
@@ -18,6 +18,11 @@ struct cpc_endpoint;
extern const struct bus_type cpc_bus;
+/* CPC endpoint flags */
+enum {
+ CPC_ENDPOINT_UP, /* Connection is established with remote counterpart. */
+};
+
/**
* struct cpc_endpoint_tcb - endpoint's transmission control block
* @lock: synchronize tcb access
@@ -37,6 +42,7 @@ struct cpc_endpoint_tcb {
u8 send_una;
u8 ack;
u8 seq;
+ u16 mtu;
};
/** struct cpc_endpoint_ops - Endpoint's callbacks.
@@ -54,6 +60,8 @@ struct cpc_endpoint_ops {
* @intf: Pointer to CPC device this endpoint belongs to.
* @list_node: list_head member for linking in a CPC device.
* @tcb: Transmission control block.
+ * @conn: Completion structure for connection.
+ * @flags: Endpoint state flags.
* @pending_ack_queue: Contain frames pending on an acknowledge.
* @holding_queue: Contains frames that were not pushed to the transport layer
* due to having insufficient space in the transmit window.
@@ -72,6 +80,8 @@ struct cpc_endpoint {
struct cpc_endpoint_ops *ops;
struct cpc_endpoint_tcb tcb;
+ struct completion conn;
+ unsigned long flags;
struct sk_buff_head pending_ack_queue;
struct sk_buff_head holding_queue;
@@ -83,6 +93,8 @@ struct cpc_endpoint *cpc_endpoint_new(struct cpc_interface *intf, u8 id, const c
void cpc_endpoint_unregister(struct cpc_endpoint *ep);
+int cpc_endpoint_connect(struct cpc_endpoint *ep);
+void cpc_endpoint_disconnect(struct cpc_endpoint *ep);
int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb);
void cpc_endpoint_set_ops(struct cpc_endpoint *ep, struct cpc_endpoint_ops *ops);
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
index db925cc078d..e6b2793d842 100644
--- a/drivers/net/cpc/endpoint.c
+++ b/drivers/net/cpc/endpoint.c
@@ -35,6 +35,7 @@ static void cpc_endpoint_tcb_reset(struct cpc_endpoint *ep)
{
ep->tcb.seq = ep->id;
ep->tcb.ack = 0;
+ ep->tcb.mtu = 0;
ep->tcb.send_nxt = ep->id;
ep->tcb.send_una = ep->id;
ep->tcb.send_wnd = 1;
@@ -72,6 +73,7 @@ struct cpc_endpoint *cpc_endpoint_alloc(struct cpc_interface *intf, u8 id)
mutex_init(&ep->tcb.lock);
cpc_endpoint_tcb_reset(ep);
+ init_completion(&ep->conn);
skb_queue_head_init(&ep->pending_ack_queue);
skb_queue_head_init(&ep->holding_queue);
@@ -197,10 +199,77 @@ void cpc_endpoint_unregister(struct cpc_endpoint *ep)
*/
void cpc_endpoint_set_ops(struct cpc_endpoint *ep, struct cpc_endpoint_ops *ops)
{
+ if (test_bit(CPC_ENDPOINT_UP, &ep->flags))
+ return;
+
if (ep)
ep->ops = ops;
}
+/**
+ * cpc_endpoint_connect - Connect to the remote endpoint.
+ * @ep: Endpoint handle.
+ *
+ * @return: 0 on success, otherwise a negative error code.
+ */
+int cpc_endpoint_connect(struct cpc_endpoint *ep)
+{
+ unsigned long timeout = msecs_to_jiffies(2000);
+ int err;
+
+ if (!ep->ops || !ep->ops->rx)
+ return -EINVAL;
+
+ if (test_bit(CPC_ENDPOINT_UP, &ep->flags))
+ return 0;
+
+ cpc_interface_add_rx_endpoint(ep);
+
+ mutex_lock(&ep->tcb.lock);
+ skb_queue_purge(&ep->pending_ack_queue);
+ skb_queue_purge(&ep->holding_queue);
+ cpc_endpoint_tcb_reset(ep);
+ mutex_unlock(&ep->tcb.lock);
+
+ err = cpc_protocol_send_syn(ep);
+ if (err)
+ goto remove_from_ep_list;
+
+ timeout = wait_for_completion_timeout(&ep->conn, timeout);
+ if (timeout == 0) {
+ err = -ETIMEDOUT;
+ mutex_lock(&ep->tcb.lock);
+ skb_queue_purge(&ep->pending_ack_queue);
+ mutex_unlock(&ep->tcb.lock);
+
+ goto remove_from_ep_list;
+ }
+
+ return 0;
+
+remove_from_ep_list:
+ cpc_interface_remove_rx_endpoint(ep);
+
+ return err;
+}
+
+/**
+ * cpc_endpoint_disconnect - Disconnect endpoint from remote.
+ * @ep: Endpoint handle.
+ *
+ * Close the connection with the remote device. When that function returns, no more packets will be
+ * received from the remote.
+ *
+ * Context: Must be called from process context, endpoint's interface lock is held.
+ */
+void cpc_endpoint_disconnect(struct cpc_endpoint *ep)
+{
+ if (!test_and_clear_bit(CPC_ENDPOINT_UP, &ep->flags))
+ return;
+
+ cpc_interface_remove_rx_endpoint(ep);
+}
+
/**
* cpc_endpoint_write - Write a DATA frame.
* @ep: Endpoint handle.
@@ -215,6 +284,11 @@ int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb)
mutex_lock(&ep->tcb.lock);
+ if (skb->len > ep->tcb.mtu) {
+ err = -EINVAL;
+ goto out;
+ }
+
if (ep->intf->ops->csum)
ep->intf->ops->csum(skb);
@@ -227,6 +301,7 @@ int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb)
err = __cpc_protocol_write(ep, &hdr, skb);
+out:
mutex_unlock(&ep->tcb.lock);
return err;
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
index edc6b387e50..d6b04588a61 100644
--- a/drivers/net/cpc/interface.c
+++ b/drivers/net/cpc/interface.c
@@ -36,6 +36,9 @@ static void cpc_interface_rx_work(struct work_struct *work)
case CPC_FRAME_TYPE_DATA:
cpc_protocol_on_data(ep, skb);
break;
+ case CPC_FRAME_TYPE_SYN:
+ cpc_protocol_on_syn(ep, skb);
+ break;
default:
kfree_skb(skb);
}
@@ -203,6 +206,32 @@ struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 e
return ep;
}
+/**
+ * cpc_interface_add_rx_endpoint() - Set an endpoint as being available for receiving frames.
+ * @ep: Endpoint.
+ */
+void cpc_interface_add_rx_endpoint(struct cpc_endpoint *ep)
+{
+ struct cpc_interface *intf = ep->intf;
+
+ mutex_lock(&intf->lock);
+ list_add_tail(&ep->list_node, &intf->eps);
+ mutex_unlock(&intf->lock);
+}
+
+/**
+ * cpc_interface_remove_rx_endpoint() - Unet an endpoint as being available for receiving frames.
+ * @ep: Endpoint.
+ */
+void cpc_interface_remove_rx_endpoint(struct cpc_endpoint *ep)
+{
+ struct cpc_interface *intf = ep->intf;
+
+ mutex_lock(&intf->lock);
+ list_del(&ep->list_node);
+ mutex_unlock(&intf->lock);
+}
+
/**
* cpc_interface_receive_frame - queue a received frame for processing
* @intf: pointer to the CPC device
diff --git a/drivers/net/cpc/interface.h b/drivers/net/cpc/interface.h
index a45227a50a7..f7f46053fad 100644
--- a/drivers/net/cpc/interface.h
+++ b/drivers/net/cpc/interface.h
@@ -68,6 +68,9 @@ void cpc_interface_unregister(struct cpc_interface *intf);
struct cpc_endpoint *cpc_interface_get_endpoint(struct cpc_interface *intf, u8 ep_id);
+void cpc_interface_add_rx_endpoint(struct cpc_endpoint *ep);
+void cpc_interface_remove_rx_endpoint(struct cpc_endpoint *ep);
+
void cpc_interface_receive_frame(struct cpc_interface *intf, struct sk_buff *skb);
void cpc_interface_send_frame(struct cpc_interface *intf, struct sk_buff *skb);
struct sk_buff *cpc_interface_dequeue(struct cpc_interface *intf);
diff --git a/drivers/net/cpc/protocol.c b/drivers/net/cpc/protocol.c
index 92e3b0a9cdf..db7ac0dc066 100644
--- a/drivers/net/cpc/protocol.c
+++ b/drivers/net/cpc/protocol.c
@@ -11,6 +11,36 @@
#include "interface.h"
#include "protocol.h"
+int cpc_protocol_send_syn(struct cpc_endpoint *ep)
+{
+ struct cpc_header hdr;
+ struct sk_buff *skb;
+ int err;
+
+ skb = cpc_skb_alloc(0, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ memset(&hdr, 0, sizeof(hdr));
+
+ mutex_lock(&ep->tcb.lock);
+
+ hdr.ctrl = cpc_header_get_ctrl(CPC_FRAME_TYPE_SYN, true);
+ hdr.ep_id = ep->id;
+ hdr.recv_wnd = CPC_HEADER_MAX_RX_WINDOW;
+ hdr.seq = ep->tcb.seq;
+ hdr.syn.mtu = cpu_to_le16(U16_MAX);
+
+ err = __cpc_protocol_write(ep, &hdr, skb);
+
+ mutex_unlock(&ep->tcb.lock);
+
+ if (err)
+ kfree_skb(skb);
+
+ return err;
+}
+
static void __cpc_protocol_send_ack(struct cpc_endpoint *ep)
{
struct cpc_header hdr;
@@ -116,6 +146,42 @@ static void __cpc_protocol_receive_ack(struct cpc_endpoint *ep, u8 recv_wnd, u8
__cpc_protocol_process_pending_tx_frames(ep);
}
+static bool __cpc_protocol_is_syn_ack_valid(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ enum cpc_frame_type type;
+ struct sk_buff *syn_skb;
+ u8 syn_seq;
+ u8 ack;
+
+ /* Fetch the previously sent frame. */
+ syn_skb = skb_peek(&ep->pending_ack_queue);
+ if (!syn_skb) {
+ dev_warn(&ep->dev, "cannot validate syn-ack, no frame was sent\n");
+ return false;
+ }
+
+ cpc_header_get_type(syn_skb->data, &type);
+
+ /* Verify if this frame is SYN. */
+ if (type != CPC_FRAME_TYPE_SYN) {
+ dev_warn(&ep->dev, "cannot validate syn-ack, no syn frame was sent (%d)\n", type);
+ return false;
+ }
+
+ syn_seq = cpc_header_get_seq(syn_skb->data);
+ ack = cpc_header_get_ack(skb->data);
+
+ /* Validate received ACK with the SEQ used in the initial SYN. */
+ if (!cpc_header_is_syn_ack_valid(syn_seq, ack)) {
+ dev_warn(&ep->dev,
+ "syn-ack (%d) is not valid with previously sent syn-seq (%d)\n",
+ ack, syn_seq);
+ return false;
+ }
+
+ return true;
+}
+
void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb)
{
bool expected_seq;
@@ -149,7 +215,7 @@ void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb)
/* Strip header. */
skb_pull(skb, CPC_HEADER_SIZE);
- if (ep->ops && ep->ops->rx)
+ if (test_bit(CPC_ENDPOINT_UP, &ep->flags))
ep->ops->rx(ep, skb);
else
kfree_skb(skb);
@@ -158,6 +224,35 @@ void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb)
}
}
+void cpc_protocol_on_syn(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ mutex_lock(&ep->tcb.lock);
+
+ if (!__cpc_protocol_is_syn_ack_valid(ep, skb))
+ goto out;
+
+ __cpc_protocol_receive_ack(ep,
+ cpc_header_get_recv_wnd(skb->data),
+ cpc_header_get_ack(skb->data));
+
+ /* On SYN-ACK, the remote's SEQ becomes our starting ACK. */
+ ep->tcb.ack = cpc_header_get_seq(skb->data);
+ ep->tcb.mtu = cpc_header_get_mtu(skb->data);
+ ep->tcb.ack++;
+
+ complete(&ep->conn);
+
+ __cpc_protocol_send_ack(ep);
+
+ set_bit(CPC_ENDPOINT_UP, &ep->flags);
+ complete(&ep->conn);
+
+out:
+ mutex_unlock(&ep->tcb.lock);
+
+ kfree_skb(skb);
+}
+
/**
* __cpc_protocol_write() - Write a frame.
* @ep: Endpoint handle.
diff --git a/drivers/net/cpc/protocol.h b/drivers/net/cpc/protocol.h
index 9a028e0e94b..e67f0f6d025 100644
--- a/drivers/net/cpc/protocol.h
+++ b/drivers/net/cpc/protocol.h
@@ -13,9 +13,13 @@
struct cpc_endpoint;
struct cpc_header;
+struct cpc_interface;
int __cpc_protocol_write(struct cpc_endpoint *ep, struct cpc_header *hdr, struct sk_buff *skb);
void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb);
+void cpc_protocol_on_syn(struct cpc_endpoint *ep, struct sk_buff *skb);
+
+int cpc_protocol_send_syn(struct cpc_endpoint *ep);
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 09/15] net: cpc: add support for RST frames
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (7 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 08/15] net: cpc: add support for connecting endpoints Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 10/15] net: cpc: make disconnect blocking Damien Riégel
` (6 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Reset frames are used to either disconnect an endpoint or to signal that
a frame is targeting an endpoint that is not connected.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/cpc.h | 1 +
drivers/net/cpc/endpoint.c | 16 ++++++++++++----
drivers/net/cpc/interface.c | 9 ++++++++-
drivers/net/cpc/protocol.c | 32 +++++++++++++++++++++++++++++++-
drivers/net/cpc/protocol.h | 2 ++
5 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
index d316fce4ad7..34ee519d907 100644
--- a/drivers/net/cpc/cpc.h
+++ b/drivers/net/cpc/cpc.h
@@ -94,6 +94,7 @@ struct cpc_endpoint *cpc_endpoint_new(struct cpc_interface *intf, u8 id, const c
void cpc_endpoint_unregister(struct cpc_endpoint *ep);
int cpc_endpoint_connect(struct cpc_endpoint *ep);
+void __cpc_endpoint_disconnect(struct cpc_endpoint *ep, bool send_rst);
void cpc_endpoint_disconnect(struct cpc_endpoint *ep);
int cpc_endpoint_write(struct cpc_endpoint *ep, struct sk_buff *skb);
void cpc_endpoint_set_ops(struct cpc_endpoint *ep, struct cpc_endpoint_ops *ops);
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
index e6b2793d842..7e2f623fb8e 100644
--- a/drivers/net/cpc/endpoint.c
+++ b/drivers/net/cpc/endpoint.c
@@ -253,6 +253,17 @@ int cpc_endpoint_connect(struct cpc_endpoint *ep)
return err;
}
+void __cpc_endpoint_disconnect(struct cpc_endpoint *ep, bool send_rst)
+{
+ if (!test_and_clear_bit(CPC_ENDPOINT_UP, &ep->flags))
+ return;
+
+ cpc_interface_remove_rx_endpoint(ep);
+
+ if (send_rst)
+ cpc_protocol_send_rst(ep->intf, ep->id);
+}
+
/**
* cpc_endpoint_disconnect - Disconnect endpoint from remote.
* @ep: Endpoint handle.
@@ -264,10 +275,7 @@ int cpc_endpoint_connect(struct cpc_endpoint *ep)
*/
void cpc_endpoint_disconnect(struct cpc_endpoint *ep)
{
- if (!test_and_clear_bit(CPC_ENDPOINT_UP, &ep->flags))
- return;
-
- cpc_interface_remove_rx_endpoint(ep);
+ __cpc_endpoint_disconnect(ep, true);
}
/**
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
index d6b04588a61..30e7976355c 100644
--- a/drivers/net/cpc/interface.c
+++ b/drivers/net/cpc/interface.c
@@ -28,6 +28,10 @@ static void cpc_interface_rx_work(struct work_struct *work)
ep = cpc_interface_get_endpoint(intf, ep_id);
if (!ep) {
+ if (type != CPC_FRAME_TYPE_RST) {
+ dev_dbg(&intf->dev, "ep%u not allocated (%d)\n", ep_id, type);
+ cpc_protocol_send_rst(intf, ep_id);
+ }
kfree_skb(skb);
continue;
}
@@ -39,8 +43,11 @@ static void cpc_interface_rx_work(struct work_struct *work)
case CPC_FRAME_TYPE_SYN:
cpc_protocol_on_syn(ep, skb);
break;
- default:
+ case CPC_FRAME_TYPE_RST:
+ dev_dbg(&ep->dev, "reset\n");
kfree_skb(skb);
+ cpc_protocol_on_rst(ep);
+ break;
}
cpc_endpoint_put(ep);
diff --git a/drivers/net/cpc/protocol.c b/drivers/net/cpc/protocol.c
index db7ac0dc066..faacd0f42ad 100644
--- a/drivers/net/cpc/protocol.c
+++ b/drivers/net/cpc/protocol.c
@@ -60,6 +60,28 @@ static void __cpc_protocol_send_ack(struct cpc_endpoint *ep)
cpc_interface_send_frame(ep->intf, skb);
}
+/**
+ * cpc_protocol_send_rst - send a RST frame
+ * @intf: interface pointer
+ * @ep_id: endpoint id
+ */
+void cpc_protocol_send_rst(struct cpc_interface *intf, u8 ep_id)
+{
+ struct cpc_header hdr = {
+ .ctrl = cpc_header_get_ctrl(CPC_FRAME_TYPE_RST, false),
+ .ep_id = ep_id,
+ };
+ struct sk_buff *skb;
+
+ skb = cpc_skb_alloc(0, GFP_KERNEL);
+ if (!skb)
+ return;
+
+ memcpy(skb_push(skb, sizeof(hdr)), &hdr, sizeof(hdr));
+
+ cpc_interface_send_frame(intf, skb);
+}
+
static void cpc_protocol_on_tx_complete(struct sk_buff *skb)
{
struct cpc_endpoint *ep = cpc_skb_get_ctx(skb);
@@ -228,8 +250,11 @@ void cpc_protocol_on_syn(struct cpc_endpoint *ep, struct sk_buff *skb)
{
mutex_lock(&ep->tcb.lock);
- if (!__cpc_protocol_is_syn_ack_valid(ep, skb))
+ if (!__cpc_protocol_is_syn_ack_valid(ep, skb)) {
+ cpc_protocol_send_rst(ep->intf, ep->id);
+
goto out;
+ }
__cpc_protocol_receive_ack(ep,
cpc_header_get_recv_wnd(skb->data),
@@ -253,6 +278,11 @@ void cpc_protocol_on_syn(struct cpc_endpoint *ep, struct sk_buff *skb)
kfree_skb(skb);
}
+void cpc_protocol_on_rst(struct cpc_endpoint *ep)
+{
+ __cpc_endpoint_disconnect(ep, false);
+}
+
/**
* __cpc_protocol_write() - Write a frame.
* @ep: Endpoint handle.
diff --git a/drivers/net/cpc/protocol.h b/drivers/net/cpc/protocol.h
index e67f0f6d025..977bb7c1450 100644
--- a/drivers/net/cpc/protocol.h
+++ b/drivers/net/cpc/protocol.h
@@ -19,7 +19,9 @@ int __cpc_protocol_write(struct cpc_endpoint *ep, struct cpc_header *hdr, struct
void cpc_protocol_on_data(struct cpc_endpoint *ep, struct sk_buff *skb);
void cpc_protocol_on_syn(struct cpc_endpoint *ep, struct sk_buff *skb);
+void cpc_protocol_on_rst(struct cpc_endpoint *ep);
+void cpc_protocol_send_rst(struct cpc_interface *intf, u8 ep_id);
int cpc_protocol_send_syn(struct cpc_endpoint *ep);
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 10/15] net: cpc: make disconnect blocking
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (8 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 09/15] net: cpc: add support for RST frames Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 11/15] net: cpc: add system endpoint Damien Riégel
` (5 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
In order to make life easier for consumer drivers, make
cpc_endpoint_disconnect() blocking. Once the call returns, it guarantees
that endpoint's rx callback won't be called anymore, making driver's
teardown simpler to implement.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/cpc.h | 1 +
drivers/net/cpc/endpoint.c | 19 ++++++++++++++++++-
drivers/net/cpc/interface.c | 4 ++++
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/net/cpc/cpc.h b/drivers/net/cpc/cpc.h
index 34ee519d907..8a761856deb 100644
--- a/drivers/net/cpc/cpc.h
+++ b/drivers/net/cpc/cpc.h
@@ -21,6 +21,7 @@ extern const struct bus_type cpc_bus;
/* CPC endpoint flags */
enum {
CPC_ENDPOINT_UP, /* Connection is established with remote counterpart. */
+ CPC_ENDPOINT_RECEIVING, /* Interface RX work is processing a frame for this endpoint. */
};
/**
diff --git a/drivers/net/cpc/endpoint.c b/drivers/net/cpc/endpoint.c
index 7e2f623fb8e..f953e4cb7ab 100644
--- a/drivers/net/cpc/endpoint.c
+++ b/drivers/net/cpc/endpoint.c
@@ -260,8 +260,25 @@ void __cpc_endpoint_disconnect(struct cpc_endpoint *ep, bool send_rst)
cpc_interface_remove_rx_endpoint(ep);
- if (send_rst)
+ if (send_rst) {
+ /*
+ * It makes sense to wait on the RECEIVING bit only when send_rst is true as this
+ * means the operation was initiated by the user and can happen concurrently with
+ * the RX work function. If a RST is received from the remote and
+ * __cpc_endpoint_disconnect from the RX work function, then it's safe to assume
+ * that this frame won't trigger a call to ep->ops->rx function.
+ */
+ int err;
+
+ err = wait_on_bit_timeout(&ep->flags,
+ CPC_ENDPOINT_RECEIVING,
+ TASK_INTERRUPTIBLE,
+ msecs_to_jiffies(1000));
+ if (!err)
+ dev_warn(&ep->dev, "Timeout when disconnecting.\n");
+
cpc_protocol_send_rst(ep->intf, ep->id);
+ }
}
/**
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
index 30e7976355c..13b52cdc357 100644
--- a/drivers/net/cpc/interface.c
+++ b/drivers/net/cpc/interface.c
@@ -36,6 +36,8 @@ static void cpc_interface_rx_work(struct work_struct *work)
continue;
}
+ set_bit(CPC_ENDPOINT_RECEIVING, &ep->flags);
+
switch (type) {
case CPC_FRAME_TYPE_DATA:
cpc_protocol_on_data(ep, skb);
@@ -50,6 +52,8 @@ static void cpc_interface_rx_work(struct work_struct *work)
break;
}
+ clear_and_wake_up_bit(CPC_ENDPOINT_RECEIVING, &ep->flags);
+
cpc_endpoint_put(ep);
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 11/15] net: cpc: add system endpoint
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (9 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 10/15] net: cpc: make disconnect blocking Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 12/15] net: cpc: create system endpoint with a new interface Damien Riégel
` (4 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
The system endpoint is the main endpoint and is guaranteed to be present
in every CPC-enabled device. It is used to communicate device
capabilities.
One of the key element of the system endpoint is that it will send a
notification when an endpoint is opened on the microcontroller. When
such notification is received by the host, a new endpoint will be
registered.
As registering a new endpoint can be a long operation, as in the end it
calls device_add(), system's endpoint processing of RX frames is
dispatched in its own work function.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/Makefile | 2 +-
drivers/net/cpc/main.c | 8 +
drivers/net/cpc/system.c | 432 +++++++++++++++++++++++++++++++++++++++
drivers/net/cpc/system.h | 14 ++
4 files changed, 455 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/cpc/system.c
create mode 100644 drivers/net/cpc/system.h
diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
index 0e9c3f775dc..a61af84df90 100644
--- a/drivers/net/cpc/Makefile
+++ b/drivers/net/cpc/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-cpc-y := endpoint.o header.o interface.o main.o protocol.o
+cpc-y := endpoint.o header.o interface.o main.o protocol.o system.o
obj-$(CONFIG_CPC) += cpc.o
diff --git a/drivers/net/cpc/main.c b/drivers/net/cpc/main.c
index 8feb0613252..fc46a25f5dc 100644
--- a/drivers/net/cpc/main.c
+++ b/drivers/net/cpc/main.c
@@ -8,6 +8,7 @@
#include "cpc.h"
#include "header.h"
+#include "system.h"
/**
* cpc_skb_alloc() - Allocate an skb with a specific headroom for CPC headers.
@@ -118,6 +119,12 @@ static int __init cpc_init(void)
int err;
err = bus_register(&cpc_bus);
+ if (err)
+ return err;
+
+ err = cpc_system_drv_register();
+ if (err)
+ bus_unregister(&cpc_bus);
return err;
}
@@ -125,6 +132,7 @@ module_init(cpc_init);
static void __exit cpc_exit(void)
{
+ cpc_system_drv_unregister();
bus_unregister(&cpc_bus);
}
module_exit(cpc_exit);
diff --git a/drivers/net/cpc/system.c b/drivers/net/cpc/system.c
new file mode 100644
index 00000000000..1d5803093f8
--- /dev/null
+++ b/drivers/net/cpc/system.c
@@ -0,0 +1,432 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/atomic.h>
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include "cpc.h"
+#include "interface.h"
+#include "protocol.h"
+#include "system.h"
+
+/**
+ * enum cpc_system_cmd_id - Describes all possible system command id's.
+ * @CPC_SYSTEM_CMD_NOOP: Used for testing purposes.
+ * @CPC_SYSTEM_CMD_PROP_NOTIFY: Used for notification purposes.
+ * @CPC_SYSTEM_CMD_PROP_VALUE_GET: Used to fetch a property.
+ * @CPC_SYSTEM_CMD_PROP_VALUE_SET: Used to set a property.
+ * @CPC_SYSTEM_CMD_PROP_VALUE_IS: Used for receiving asynchronous properties.
+ * @CPC_SYSTEM_CMD_INVALID: Used to indicate a system command was invalid.
+ */
+enum cpc_system_cmd_id {
+ CPC_SYSTEM_CMD_NOOP = 0x00,
+ CPC_SYSTEM_CMD_PROP_NOTIFY = 0x01,
+ CPC_SYSTEM_CMD_PROP_VALUE_GET = 0x02,
+ CPC_SYSTEM_CMD_PROP_VALUE_SET = 0x03,
+ CPC_SYSTEM_CMD_PROP_VALUE_IS = 0x06,
+ CPC_SYSTEM_CMD_INVALID = 0xFF,
+};
+
+/**
+ * enum cpc_system_property_id - Describes all possible system property identifiers.
+ * @CPC_SYSTEM_PROP_INVALID: Invalid property.
+ * @CPC_SYSTEM_PROP_PROTOCOL_VERSION: Protocol version property.
+ * @CPC_SYSTEM_PROP_DRV_CAPABILITIES: Driver capabilities property.
+ * @CPC_SYSTEM_PROP_CPC_VERSION: CPC version property.
+ * @CPC_SYSTEM_PROP_APP_VERSION: Application version property.
+ * @CPC_SYSTEM_PROP_RESET_REASON: Reset reason property.
+ * @CPC_SYSTEM_PROP_EP_OPEN_NOTIF: Endpoint open notification property.
+ */
+enum cpc_system_property_id {
+ CPC_SYSTEM_PROP_INVALID = 0x00,
+ CPC_SYSTEM_PROP_PROTOCOL_VERSION = 0x10,
+ CPC_SYSTEM_PROP_DRV_CAPABILITIES = 0x11,
+ CPC_SYSTEM_PROP_CPC_VERSION = 0x12,
+ CPC_SYSTEM_PROP_APP_VERSION = 0x13,
+ CPC_SYSTEM_PROP_RESET_REASON = 0x14,
+ CPC_SYSTEM_PROP_EP_OPEN_NOTIF = 0x800,
+};
+
+/**
+ * enum cpc_system_status - Describes all possible system statuses.
+ * @CPC_SYSTEM_STATUS_RESET_POWER_ON: Reset was due to a power reason.
+ * @CPC_SYSTEM_STATUS_RESET_EXTERNAL: Reset was due to an external reason.
+ * @CPC_SYSTEM_STATUS_RESET_SOFTWARE: Reset was due to a software reason.
+ * @CPC_SYSTEM_STATUS_RESET_FAULT: Reset was due to a fault.
+ * @CPC_SYSTEM_STATUS_RESET_CRASH: Reset was due to a crash.
+ * @CPC_SYSTEM_STATUS_RESET_ASSERT: Reset was due to an assert.
+ * @CPC_SYSTEM_STATUS_RESET_OTHER: Reset was due to some other reason.
+ * @CPC_SYSTEM_STATUS_RESET_UNKNOWN: Reset was due to an unknown reason.
+ * @CPC_SYSTEM_STATUS_RESET_WATCHDOG: Reset was due to a watchdog trigger.
+ */
+enum cpc_system_status {
+ CPC_SYSTEM_STATUS_RESET_POWER_ON = 112,
+ CPC_SYSTEM_STATUS_RESET_EXTERNAL = 113,
+ CPC_SYSTEM_STATUS_RESET_SOFTWARE = 114,
+ CPC_SYSTEM_STATUS_RESET_FAULT = 115,
+ CPC_SYSTEM_STATUS_RESET_CRASH = 116,
+ CPC_SYSTEM_STATUS_RESET_ASSERT = 117,
+ CPC_SYSTEM_STATUS_RESET_OTHER = 118,
+ CPC_SYSTEM_STATUS_RESET_UNKNOWN = 119,
+ CPC_SYSTEM_STATUS_RESET_WATCHDOG = 120,
+};
+
+/**
+ * struct cpc_system_cmd - Wire representation of the system command.
+ * @id: Identifier of the command.
+ * @seq: Command sequence number.
+ * @len: Length of the payload in bytes.
+ * @payload: Variable length payload.
+ */
+struct cpc_system_cmd {
+ u8 id;
+ u8 seq;
+ __le16 len;
+ u8 payload[];
+} __packed;
+
+/**
+ * struct cpc_system_cmd_property - Wire representation of the system
+ * command property.
+ * @id: Identifier of the property.
+ * @payload: Variable length property value.
+ */
+struct cpc_system_cmd_property {
+ __le32 id;
+ u8 payload[];
+} __packed;
+
+struct cpc_system_cmd_handle {
+ u8 seq;
+ struct list_head node;
+};
+
+struct cpc_system_cmd_ctx {
+ struct mutex lock; /* Synchronize access to command list */
+ u8 next_seq;
+ struct list_head list;
+
+ struct work_struct init_work;
+ struct work_struct rx_work;
+
+ struct sk_buff_head rx_queue;
+
+ struct cpc_endpoint *ep;
+};
+
+static bool __cpc_system_cmd_pop_handle(struct cpc_system_cmd_ctx *cmd_ctx,
+ struct cpc_system_cmd_handle **cmd_handle)
+{
+ *cmd_handle = list_first_entry_or_null(&cmd_ctx->list, struct cpc_system_cmd_handle, node);
+
+ if (*cmd_handle)
+ list_del(&(*cmd_handle)->node);
+
+ return !!*cmd_handle;
+}
+
+static void cpc_system_cmd_init(struct cpc_system_cmd_ctx *cmd_ctx)
+{
+ mutex_init(&cmd_ctx->lock);
+ INIT_LIST_HEAD(&cmd_ctx->list);
+ cmd_ctx->next_seq = 0;
+}
+
+static void cpc_system_cmd_clear(struct cpc_system_cmd_ctx *cmd_ctx)
+{
+ struct cpc_system_cmd_handle *cmd_handle;
+
+ mutex_lock(&cmd_ctx->lock);
+ while (__cpc_system_cmd_pop_handle(cmd_ctx, &cmd_handle))
+ kfree(cmd_handle);
+
+ cmd_ctx->next_seq = 0;
+ mutex_unlock(&cmd_ctx->lock);
+}
+
+static bool cpc_system_cmd_find_handle(struct cpc_system_cmd_ctx *cmd_ctx,
+ u8 cmd_seq,
+ struct cpc_system_cmd_handle **cmd_handle)
+{
+ mutex_lock(&cmd_ctx->lock);
+
+ list_for_each_entry((*cmd_handle), &cmd_ctx->list, node) {
+ if ((*cmd_handle)->seq == cmd_seq) {
+ list_del(&(*cmd_handle)->node);
+ mutex_unlock(&cmd_ctx->lock);
+ return true;
+ }
+ }
+
+ mutex_unlock(&cmd_ctx->lock);
+
+ return false;
+}
+
+static void cpc_system_on_reset_reason(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ u32 reset_reason;
+
+ if (skb->len != sizeof(reset_reason)) {
+ dev_err(&ep->dev, "reset reason has invalid length (%d)\n", skb->len);
+ return;
+ }
+
+ reset_reason = get_unaligned_le32(skb->data);
+
+ switch (reset_reason) {
+ case CPC_SYSTEM_STATUS_RESET_POWER_ON:
+ case CPC_SYSTEM_STATUS_RESET_EXTERNAL:
+ case CPC_SYSTEM_STATUS_RESET_SOFTWARE:
+ case CPC_SYSTEM_STATUS_RESET_FAULT:
+ case CPC_SYSTEM_STATUS_RESET_CRASH:
+ case CPC_SYSTEM_STATUS_RESET_ASSERT:
+ case CPC_SYSTEM_STATUS_RESET_OTHER:
+ case CPC_SYSTEM_STATUS_RESET_UNKNOWN:
+ case CPC_SYSTEM_STATUS_RESET_WATCHDOG:
+ dev_dbg(&ep->dev, "reset reason: %d\n", reset_reason);
+ break;
+ default:
+ dev_dbg(&ep->dev, "undefined reset reason: %d\n", reset_reason);
+ break;
+ }
+}
+
+static void cpc_system_prop_value_is(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ u32 id;
+
+ if (skb->len < sizeof(id)) {
+ dev_warn(&ep->dev, "command property with invalid length (%d)\n", skb->len);
+ return;
+ }
+
+ id = get_unaligned_le32(skb_pull_data(skb, sizeof(id)));
+
+ switch (id) {
+ case CPC_SYSTEM_PROP_RESET_REASON:
+ cpc_system_on_reset_reason(ep, skb);
+ break;
+ default:
+ dev_dbg(&ep->dev, "unsupported command property identifier (%d)\n", id);
+ break;
+ }
+}
+
+static void cpc_system_on_prop_value_is(struct cpc_endpoint *ep, struct sk_buff *skb, u8 seq)
+{
+ struct cpc_system_cmd_ctx *cmd_ctx = cpc_endpoint_get_drvdata(ep);
+ struct cpc_system_cmd_handle *cmd_handle;
+
+ if (cpc_system_cmd_find_handle(cmd_ctx, seq, &cmd_handle)) {
+ cpc_system_prop_value_is(ep, skb);
+ kfree(cmd_handle);
+ } else {
+ dev_warn(&ep->dev, "unknown command sequence (%u)\n", seq);
+ }
+}
+
+static void cpc_system_on_ep_open_notification(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ struct cpc_endpoint *new_ep;
+ u8 ep_id;
+
+ /*
+ * Payload should contain at least the endpoint ID
+ * and the null terminating byte of the string.
+ */
+ if (skb->len < sizeof(ep_id) + 1) {
+ dev_warn(&ep->dev, "open with invalid length (%d)\n", skb->len);
+ return;
+ }
+
+ ep_id = *(u8 *)skb_pull_data(skb, sizeof(ep_id));
+
+ if (skb->data[skb->len - 1] != '\0') {
+ dev_warn(&ep->dev, "non nul-terminated endpoint name for ep%u\n", ep_id);
+ return;
+ }
+
+ if (skb->len > sizeof(ep->name)) {
+ dev_warn(&ep->dev, "oversized endpoint name (%d) for ep%u\n", skb->len, ep_id);
+ return;
+ }
+
+ dev_dbg(&ep->dev, "open notification for ep%u: '%s'\n", ep_id, skb->data);
+
+ new_ep = cpc_endpoint_alloc(ep->intf, ep_id);
+ if (!new_ep)
+ return;
+
+ memcpy(new_ep->name, skb->data, skb->len);
+
+ /* Register the new endpoint in the same interface that received the notification. */
+ cpc_endpoint_register(new_ep);
+}
+
+static void cpc_system_on_prop_notify(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ u32 id;
+
+ if (skb->len < sizeof(id)) {
+ dev_warn(&ep->dev, "command property with invalid length (%d)\n", skb->len);
+ return;
+ }
+
+ id = get_unaligned_le32(skb_pull_data(skb, sizeof(id)));
+
+ switch (id) {
+ case CPC_SYSTEM_PROP_EP_OPEN_NOTIF:
+ cpc_system_on_ep_open_notification(ep, skb);
+ break;
+ default:
+ dev_dbg(&ep->dev, "unsupported command property identifier (%d)\n", id);
+ break;
+ }
+}
+
+static void cpc_system_on_data(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ struct cpc_system_cmd *system_cmd;
+ u16 payload_len;
+ u8 seq;
+
+ if (skb->len < sizeof(*system_cmd)) {
+ dev_warn(&ep->dev, "command with invalid length (%d)\n", skb->len);
+ kfree_skb(skb);
+ return;
+ }
+
+ system_cmd = skb_pull_data(skb, sizeof(*system_cmd));
+ payload_len = le16_to_cpu(system_cmd->len);
+ seq = system_cmd->seq;
+
+ if (skb->len != payload_len) {
+ dev_warn(&ep->dev,
+ "command payload length does not match (%d != %d)\n",
+ skb->len, payload_len);
+ kfree_skb(skb);
+ return;
+ }
+
+ switch (system_cmd->id) {
+ case CPC_SYSTEM_CMD_PROP_VALUE_IS:
+ cpc_system_on_prop_value_is(ep, skb, seq);
+ break;
+ case CPC_SYSTEM_CMD_PROP_NOTIFY:
+ cpc_system_on_prop_notify(ep, skb);
+ break;
+ default:
+ dev_dbg(&ep->dev, "unsupported system command identifier (%d)\n", system_cmd->id);
+ break;
+ }
+
+ kfree_skb(skb);
+}
+
+static void cpc_system_rx_work(struct work_struct *work)
+{
+ struct cpc_system_cmd_ctx *cmd_ctx = container_of(work, struct cpc_system_cmd_ctx, rx_work);
+ struct sk_buff *skb;
+
+ while ((skb = skb_dequeue(&cmd_ctx->rx_queue)))
+ cpc_system_on_data(cmd_ctx->ep, skb);
+}
+
+static void cpc_system_rx(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ struct cpc_system_cmd_ctx *cmd_ctx = cpc_endpoint_get_drvdata(ep);
+
+ skb_queue_tail(&cmd_ctx->rx_queue, skb);
+ schedule_work(&cmd_ctx->rx_work);
+}
+
+static void cpc_system_init_work(struct work_struct *work)
+{
+ struct cpc_system_cmd_ctx *cmd_ctx = container_of(work,
+ struct cpc_system_cmd_ctx,
+ init_work);
+
+ cpc_endpoint_connect(cmd_ctx->ep);
+}
+
+static struct cpc_endpoint_ops system_ops = {
+ .rx = cpc_system_rx,
+};
+
+static int cpc_system_probe(struct cpc_endpoint *ep)
+{
+ struct cpc_system_cmd_ctx *cmd_ctx;
+
+ cmd_ctx = kzalloc(sizeof(*cmd_ctx), GFP_KERNEL);
+ if (!cmd_ctx)
+ return -ENOMEM;
+
+ cpc_system_cmd_init(cmd_ctx);
+
+ INIT_WORK(&cmd_ctx->init_work, cpc_system_init_work);
+ INIT_WORK(&cmd_ctx->rx_work, cpc_system_rx_work);
+
+ skb_queue_head_init(&cmd_ctx->rx_queue);
+
+ cmd_ctx->ep = ep;
+
+ cpc_endpoint_set_drvdata(ep, cmd_ctx);
+ cpc_endpoint_set_ops(ep, &system_ops);
+
+ /* Defer connection of the system endpoint to not block in probe(). */
+ schedule_work(&cmd_ctx->init_work);
+
+ return 0;
+}
+
+static void cpc_system_remove(struct cpc_endpoint *ep)
+{
+ struct cpc_system_cmd_ctx *cmd_ctx = cpc_endpoint_get_drvdata(ep);
+
+ cpc_endpoint_disconnect(ep);
+
+ cpc_system_cmd_clear(cmd_ctx);
+
+ flush_work(&cmd_ctx->init_work);
+ flush_work(&cmd_ctx->rx_work);
+
+ skb_queue_purge(&cmd_ctx->rx_queue);
+
+ kfree(cmd_ctx);
+}
+
+static struct cpc_driver system_driver = {
+ .driver = {
+ .name = CPC_SYSTEM_ENDPOINT_NAME,
+ },
+ .probe = cpc_system_probe,
+ .remove = cpc_system_remove,
+};
+
+/**
+ * cpc_system_drv_register - Register the system endpoint driver.
+ *
+ * @return: 0 on success, otherwise a negative error code.
+ */
+int cpc_system_drv_register(void)
+{
+ return cpc_driver_register(&system_driver);
+}
+
+/**
+ * cpc_system_drv_unregister - Unregister the system endpoint driver.
+ */
+void cpc_system_drv_unregister(void)
+{
+ cpc_driver_unregister(&system_driver);
+}
diff --git a/drivers/net/cpc/system.h b/drivers/net/cpc/system.h
new file mode 100644
index 00000000000..31307da3547
--- /dev/null
+++ b/drivers/net/cpc/system.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_SYSTEM_H
+#define __CPC_SYSTEM_H
+
+#define CPC_SYSTEM_ENDPOINT_NAME "system"
+
+int cpc_system_drv_register(void);
+void cpc_system_drv_unregister(void);
+
+#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 12/15] net: cpc: create system endpoint with a new interface
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (10 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 11/15] net: cpc: add system endpoint Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 1:27 ` [RFC net-next 13/15] dt-bindings: net: cpc: add silabs,cpc-spi.yaml Damien Riégel
` (3 subsequent siblings)
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Without a system endpoint, no new endpoints will ever be created on an
interface, so as part of the registration of a new interface, create the
system endpoint attached to it.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/interface.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/net/cpc/interface.c b/drivers/net/cpc/interface.c
index 13b52cdc357..dec4c9be3c4 100644
--- a/drivers/net/cpc/interface.c
+++ b/drivers/net/cpc/interface.c
@@ -9,6 +9,7 @@
#include "header.h"
#include "interface.h"
#include "protocol.h"
+#include "system.h"
#define to_cpc_interface(d) container_of(d, struct cpc_interface, dev)
@@ -143,13 +144,25 @@ struct cpc_interface *cpc_interface_alloc(struct device *parent,
*/
int cpc_interface_register(struct cpc_interface *intf)
{
+ struct cpc_endpoint *system_ep;
int err;
err = device_add(&intf->dev);
if (err)
return err;
+ system_ep = cpc_endpoint_new(intf, 0, CPC_SYSTEM_ENDPOINT_NAME);
+ if (!system_ep) {
+ err = -ENOMEM;
+ goto unregister_intf;
+ }
+
return 0;
+
+unregister_intf:
+ cpc_interface_unregister(intf);
+
+ return err;
}
static int cpc_intf_unregister_ep(struct device *dev, void *null)
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC net-next 13/15] dt-bindings: net: cpc: add silabs,cpc-spi.yaml
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (11 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 12/15] net: cpc: create system endpoint with a new interface Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-14 21:38 ` Rob Herring
2025-05-12 1:27 ` [RFC net-next 14/15] net: cpc: add SPI interface driver Damien Riégel
` (2 subsequent siblings)
15 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Document device tree bindings for Silicon Labs CPC over a SPI bus. This
device requires both a chip select and an interrupt line to be able to
work.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
.../bindings/net/silabs,cpc-spi.yaml | 54 +++++++++++++++++++
1 file changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml
diff --git a/Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml b/Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml
new file mode 100644
index 00000000000..82d3cd47daa
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2024 Silicon Labs Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/silabs,cpc-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SPI driver for CPC
+
+maintainers:
+ - Damien Riégel <damien.riegel@silabs.com>
+
+description: |
+ This binding is for the implementation of CPC protocol over SPI. The protocol
+ consists of a chain of header+payload frames. The interrupt is used by the
+ device to signal it has a frame to transmit, but also between headers and
+ payloads to signal that it is ready to receive payload.
+
+properties:
+ compatible:
+ enum:
+ - silabs,cpc-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupt
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpcspi@0 {
+ compatible = "silabs,cpc-spi";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ interrupt-parent = <&gpio>;
+ interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
+ };
+ };
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC net-next 13/15] dt-bindings: net: cpc: add silabs,cpc-spi.yaml
2025-05-12 1:27 ` [RFC net-next 13/15] dt-bindings: net: cpc: add silabs,cpc-spi.yaml Damien Riégel
@ 2025-05-14 21:38 ` Rob Herring
0 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2025-05-14 21:38 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
On Sun, May 11, 2025 at 09:27:46PM -0400, Damien Riégel wrote:
> Document device tree bindings for Silicon Labs CPC over a SPI bus. This
> device requires both a chip select and an interrupt line to be able to
> work.
What's CPC? Never defined here.
Bindings are for devices, not a SPI protocol. What if the device needs
reset or power or ??? before you can talk to it. Maybe it's a situation
where that will never matter, but you've got to spell it out here.
>
> Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
> ---
> .../bindings/net/silabs,cpc-spi.yaml | 54 +++++++++++++++++++
> 1 file changed, 54 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml b/Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml
> new file mode 100644
> index 00000000000..82d3cd47daa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/silabs,cpc-spi.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2024 Silicon Labs Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/silabs,cpc-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SPI driver for CPC
> +
> +maintainers:
> + - Damien Riégel <damien.riegel@silabs.com>
> +
> +description: |
Don't need '|'
> + This binding is for the implementation of CPC protocol over SPI. The protocol
> + consists of a chain of header+payload frames. The interrupt is used by the
> + device to signal it has a frame to transmit, but also between headers and
> + payloads to signal that it is ready to receive payload.
> +
> +properties:
> + compatible:
> + enum:
> + - silabs,cpc-spi
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupt
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpcspi@0 {
> + compatible = "silabs,cpc-spi";
> + reg = <0>;
> + spi-max-frequency = <1000000>;
> + interrupt-parent = <&gpio>;
> + interrupts = <23 IRQ_TYPE_EDGE_FALLING>;
> + };
> + };
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC net-next 14/15] net: cpc: add SPI interface driver
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (12 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 13/15] dt-bindings: net: cpc: add silabs,cpc-spi.yaml Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 2:47 ` Andrew Lunn
2025-05-12 1:27 ` [RFC net-next 15/15] net: cpc: add Bluetooth HCI driver Damien Riégel
2025-05-12 17:07 ` [RFC net-next 00/15] Add support for Silicon Labs CPC Andrew Lunn
15 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
This adds support for CPC over SPI. CPC uses a full-duplex protocol.
Each frame transmission/reception is split into two parts:
- read and/or write header + header checksum
- read and/or write payload + payload checksum
Header frames are always 10 bytes (8 bytes of header and 2 bytes of
checksum). The header contains the size of the payload to receive (size
to transmit is already known). As the SPI device also has some
processing to do when it receives a header, the SPI driver must wait for
the interrupt line to be asserted before clocking the payload.
The SPI device always expects the chip select to be asserted and
deasserted after a header, even if there are no payloads to transmit.
This is used to keep headers transmission synchronized between host and
device. As some controllers don't support doing that if there is nothing
to transmit, a null byte is transmitted in that case and it will be
ignored by the device.
If there are payloads, the driver will clock the maximum length of the
two payloads.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/Kconfig | 3 +-
drivers/net/cpc/Makefile | 2 +-
drivers/net/cpc/main.c | 8 +
drivers/net/cpc/spi.c | 550 +++++++++++++++++++++++++++++++++++++++
drivers/net/cpc/spi.h | 12 +
5 files changed, 573 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/cpc/spi.c
create mode 100644 drivers/net/cpc/spi.h
diff --git a/drivers/net/cpc/Kconfig b/drivers/net/cpc/Kconfig
index f31b6837b49..f5159390a82 100644
--- a/drivers/net/cpc/Kconfig
+++ b/drivers/net/cpc/Kconfig
@@ -2,7 +2,8 @@
menuconfig CPC
tristate "Silicon Labs Co-Processor Communication (CPC) Protocol"
- depends on NET
+ depends on NET && SPI
+ select CRC_ITU_T
help
Provide support for the CPC protocol to Silicon Labs EFR32 devices.
diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
index a61af84df90..195cdf4ad62 100644
--- a/drivers/net/cpc/Makefile
+++ b/drivers/net/cpc/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-cpc-y := endpoint.o header.o interface.o main.o protocol.o system.o
+cpc-y := endpoint.o header.o interface.o main.o protocol.o spi.o system.o
obj-$(CONFIG_CPC) += cpc.o
diff --git a/drivers/net/cpc/main.c b/drivers/net/cpc/main.c
index fc46a25f5dc..b4e73145ac2 100644
--- a/drivers/net/cpc/main.c
+++ b/drivers/net/cpc/main.c
@@ -8,6 +8,7 @@
#include "cpc.h"
#include "header.h"
+#include "spi.h"
#include "system.h"
/**
@@ -126,12 +127,19 @@ static int __init cpc_init(void)
if (err)
bus_unregister(&cpc_bus);
+ err = cpc_spi_register_driver();
+ if (err) {
+ cpc_system_drv_unregister();
+ bus_unregister(&cpc_bus);
+ }
+
return err;
}
module_init(cpc_init);
static void __exit cpc_exit(void)
{
+ cpc_spi_unregister_driver();
cpc_system_drv_unregister();
bus_unregister(&cpc_bus);
}
diff --git a/drivers/net/cpc/spi.c b/drivers/net/cpc/spi.c
new file mode 100644
index 00000000000..2b068eeb5d4
--- /dev/null
+++ b/drivers/net/cpc/spi.c
@@ -0,0 +1,550 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/atomic.h>
+#include <linux/crc-itu-t.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/kthread.h>
+#include <linux/minmax.h>
+#include <linux/of.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/unaligned.h>
+#include <linux/wait.h>
+
+#include "cpc.h"
+#include "header.h"
+#include "interface.h"
+#include "spi.h"
+
+#define CPC_SPI_CSUM_SIZE 2
+#define CPC_SPI_INTERRUPT_MAX_WAIT_MS 1000
+#define CPC_SPI_MAX_PAYLOAD_SIZE 4096
+
+struct cpc_spi {
+ struct spi_device *spi;
+ struct cpc_interface *intf;
+
+ struct task_struct *task;
+ wait_queue_head_t event_queue;
+
+ struct sk_buff *tx_skb;
+ u8 tx_csum[CPC_SPI_CSUM_SIZE];
+
+ atomic_t event_cond;
+ struct sk_buff *rx_skb;
+ unsigned int rx_len;
+ u8 rx_header[CPC_HEADER_SIZE + CPC_SPI_CSUM_SIZE];
+};
+
+static bool buffer_is_zeroes(const u8 *buffer, size_t length)
+{
+ for (size_t i = 0; i < length; i++) {
+ if (buffer[i] != 0)
+ return false;
+ }
+
+ return true;
+}
+
+static u16 cpc_spi_csum(const u8 *buffer, size_t length)
+{
+ return crc_itu_t(0, buffer, length);
+}
+
+static int cpc_spi_do_xfer_header(struct cpc_spi *ctx)
+{
+ struct spi_transfer xfer_header = {
+ .rx_buf = ctx->rx_header,
+ .len = CPC_HEADER_SIZE,
+ .speed_hz = ctx->spi->max_speed_hz,
+ };
+ struct spi_transfer xfer_csum = {
+ .rx_buf = &ctx->rx_header[CPC_HEADER_SIZE],
+ .len = sizeof(ctx->tx_csum),
+ .speed_hz = ctx->spi->max_speed_hz,
+ };
+ enum cpc_frame_type type;
+ struct spi_message msg;
+ size_t payload_len = 0;
+ struct sk_buff *skb;
+ u16 rx_csum;
+ u16 csum;
+ int ret;
+
+ if (ctx->tx_skb) {
+ u16 tx_hdr_csum = cpc_spi_csum(ctx->tx_skb->data, CPC_HEADER_SIZE);
+
+ put_unaligned_le16(tx_hdr_csum, ctx->tx_csum);
+
+ xfer_header.tx_buf = ctx->tx_skb->data;
+ xfer_csum.tx_buf = ctx->tx_csum;
+ }
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&xfer_header, &msg);
+ spi_message_add_tail(&xfer_csum, &msg);
+
+ ret = spi_sync(ctx->spi, &msg);
+ if (ret)
+ return ret;
+
+ if (ctx->tx_skb) {
+ if (skb_headlen(ctx->tx_skb) == CPC_HEADER_SIZE) {
+ kfree_skb(ctx->tx_skb);
+ ctx->tx_skb = NULL;
+ } else {
+ skb_pull(ctx->tx_skb, CPC_HEADER_SIZE);
+ }
+ }
+
+ if (buffer_is_zeroes(ctx->rx_header, CPC_HEADER_SIZE))
+ return 0;
+
+ rx_csum = get_unaligned_le16(&ctx->rx_header[CPC_HEADER_SIZE]);
+ csum = cpc_spi_csum(ctx->rx_header, CPC_HEADER_SIZE);
+
+ if (rx_csum != csum || !cpc_header_get_type(ctx->rx_header, &type)) {
+ /*
+ * If the header checksum is invalid, its length can't be trusted, receive
+ * the maximum payload length to recover from that situation. If the frame
+ * type cannot be extracted from the header, use same recovery mechanism.
+ */
+ ctx->rx_len = CPC_SPI_MAX_PAYLOAD_SIZE;
+
+ return 0;
+ }
+
+ if (type == CPC_FRAME_TYPE_DATA)
+ payload_len = cpc_header_get_payload_len(ctx->rx_header) +
+ sizeof(ctx->tx_csum);
+
+ skb = cpc_skb_alloc(payload_len, GFP_KERNEL);
+ if (!skb) {
+ /*
+ * Failed to allocate memory to receive the payload. Driver must clock in
+ * these bytes even if there is no room, to keep the sender in sync.
+ */
+ ctx->rx_len = payload_len;
+
+ return 0;
+ }
+
+ memcpy(skb_push(skb, CPC_HEADER_SIZE), ctx->rx_header, CPC_HEADER_SIZE);
+
+ if (payload_len) {
+ ctx->rx_skb = skb;
+ ctx->rx_len = payload_len;
+ } else {
+ cpc_interface_receive_frame(ctx->intf, skb);
+ }
+
+ return 0;
+}
+
+static int cpc_spi_do_xfer_notch(struct cpc_spi *ctx)
+{
+ struct spi_transfer xfer = {
+ .tx_buf = ctx->tx_csum,
+ .len = 1,
+ .speed_hz = ctx->spi->max_speed_hz,
+ };
+ struct spi_message msg;
+
+ ctx->tx_csum[0] = 0;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&xfer, &msg);
+
+ return spi_sync(ctx->spi, &msg);
+}
+
+static int cpc_spi_do_xfer_payload(struct cpc_spi *ctx)
+{
+ struct spi_transfer shared_xfer = {
+ .speed_hz = ctx->spi->max_speed_hz,
+ .rx_buf = NULL,
+ .tx_buf = NULL,
+ };
+ struct spi_transfer pad_xfer1 = {
+ .speed_hz = ctx->spi->max_speed_hz,
+ .rx_buf = NULL,
+ .tx_buf = NULL,
+ };
+ struct spi_transfer pad_xfer2 = {
+ .speed_hz = ctx->spi->max_speed_hz,
+ .rx_buf = NULL,
+ .tx_buf = NULL,
+ };
+ unsigned int rx_len = ctx->rx_len;
+ unsigned int tx_data_len;
+ struct spi_message msg;
+ int ret;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&shared_xfer, &msg);
+
+ /*
+ * This can happen if header checksum was invalid. In that case, protocol
+ * mandates to be ready to receive the maximum number of bytes that the
+ * device is capable to send, in order to be sure its TX queue is flushed.
+ */
+ if (!ctx->rx_skb && rx_len) {
+ shared_xfer.rx_buf = kmalloc(rx_len, GFP_KERNEL);
+ if (!shared_xfer.rx_buf)
+ return -ENOMEM;
+
+ shared_xfer.len = rx_len;
+ }
+
+ if (ctx->rx_skb && !ctx->tx_skb) {
+ shared_xfer.rx_buf = skb_put(ctx->rx_skb, rx_len);
+ shared_xfer.len = rx_len;
+ }
+
+ if (ctx->tx_skb) {
+ u16 csum = ctx->tx_skb->csum;
+
+ put_unaligned_le16(csum, ctx->tx_csum);
+
+ tx_data_len = ctx->tx_skb->len;
+
+ shared_xfer.tx_buf = ctx->tx_skb->data;
+ shared_xfer.len = tx_data_len;
+
+ if (!ctx->rx_skb) {
+ pad_xfer1.tx_buf = ctx->tx_csum;
+ pad_xfer1.len = sizeof(ctx->tx_csum);
+
+ spi_message_add_tail(&pad_xfer1, &msg);
+ }
+ }
+
+ if (ctx->rx_skb && ctx->tx_skb) {
+ unsigned int shared_len;
+ unsigned int pad_len;
+
+ shared_len = min(rx_len, tx_data_len);
+ pad_len = max(rx_len, tx_data_len) - shared_len;
+
+ shared_xfer.rx_buf = skb_put(ctx->rx_skb, shared_len);
+ shared_xfer.len = shared_len;
+
+ if (rx_len < tx_data_len) {
+ /*
+ * |------- RX BUFFER + RX CSUM ------|
+ * |------------------- TX BUFFER ------------|---- TX CSUM ----|
+ *
+ * | SHARED |
+ * | PAD 1 |
+ * | PAD 2 |
+ */
+ pad_xfer1.rx_buf = NULL;
+ pad_xfer1.tx_buf = ctx->tx_skb->data + shared_len;
+ pad_xfer1.len = pad_len;
+
+ pad_xfer2.rx_buf = NULL;
+ pad_xfer2.tx_buf = ctx->tx_csum;
+ pad_xfer2.len = sizeof(ctx->tx_csum);
+
+ spi_message_add_tail(&pad_xfer1, &msg);
+ spi_message_add_tail(&pad_xfer2, &msg);
+ } else if (rx_len == tx_data_len) {
+ /*
+ * |------------- RX BUFFER + RX CSUM ---------|
+ * |------------------- TX BUFFER -------------|---- TX CSUM ---|
+ *
+ * | SHARED |
+ * | PAD 1 |
+ */
+ pad_xfer1.rx_buf = NULL;
+ pad_xfer1.tx_buf = ctx->tx_csum;
+ pad_xfer1.len = sizeof(ctx->tx_csum);
+
+ spi_message_add_tail(&pad_xfer1, &msg);
+ } else if (rx_len == tx_data_len + 1) {
+ /*
+ * |----------------- RX BUFFER + RX CSUM ----------------|
+ * |------------------- TX BUFFER -------------|---- TX CSUM ---|
+ *
+ * | SHARED |
+ * | PAD 1 |
+ * | PAD 2 |
+ */
+ pad_xfer1.tx_buf = ctx->tx_csum;
+ pad_xfer1.rx_buf = skb_put(ctx->rx_skb, 1);
+ pad_xfer1.len = 1;
+
+ pad_xfer2.tx_buf = &ctx->tx_csum[1];
+ pad_xfer2.rx_buf = NULL;
+ pad_xfer2.len = 1;
+
+ spi_message_add_tail(&pad_xfer1, &msg);
+ spi_message_add_tail(&pad_xfer2, &msg);
+ } else {
+ /*
+ * |----------------------------- RX BUFFER + RX CSUM -------------------|
+ * |------------------- TX BUFFER -------------|---- TX CSUM ---|
+ *
+ * | SHARED |
+ * | PAD 1 |
+ * | PAD 2 |
+ */
+ pad_xfer1.tx_buf = ctx->tx_csum;
+ pad_xfer1.rx_buf = skb_put(ctx->rx_skb, sizeof(ctx->tx_csum));
+ pad_xfer1.len = sizeof(ctx->tx_csum);
+
+ pad_xfer2.tx_buf = NULL;
+ pad_xfer2.rx_buf = skb_put(ctx->rx_skb, pad_len - sizeof(ctx->tx_csum));
+ pad_xfer2.len = pad_len - sizeof(ctx->tx_csum);
+
+ spi_message_add_tail(&pad_xfer1, &msg);
+ spi_message_add_tail(&pad_xfer2, &msg);
+ }
+ }
+
+ ret = spi_sync(ctx->spi, &msg);
+
+ if (ctx->tx_skb) {
+ kfree_skb(ctx->tx_skb);
+ ctx->tx_skb = NULL;
+ }
+
+ if (ctx->rx_skb) {
+ unsigned char *csum_ptr;
+ u16 expected_csum;
+ u16 csum;
+
+ if (ret) {
+ kfree_skb(ctx->rx_skb);
+ goto exit;
+ }
+
+ csum_ptr = skb_tail_pointer(ctx->rx_skb) - sizeof(csum);
+ csum = get_unaligned_le16(csum_ptr);
+
+ expected_csum = cpc_spi_csum(ctx->rx_skb->data + CPC_HEADER_SIZE,
+ ctx->rx_len - sizeof(csum));
+
+ if (csum == expected_csum) {
+ skb_trim(ctx->rx_skb, ctx->rx_skb->len - sizeof(csum));
+
+ cpc_interface_receive_frame(ctx->intf, ctx->rx_skb);
+ } else {
+ kfree_skb(ctx->rx_skb);
+ }
+ }
+
+exit:
+ ctx->rx_skb = NULL;
+ ctx->rx_len = 0;
+
+ return ret;
+}
+
+static int cpc_spi_do_xfer_thread(void *data)
+{
+ struct cpc_spi *ctx = data;
+ bool xfer_idle = true;
+ int ret;
+
+ while (!kthread_should_stop()) {
+ if (xfer_idle) {
+ ret = wait_event_interruptible(ctx->event_queue,
+ (!cpc_interface_tx_queue_empty(ctx->intf) ||
+ atomic_read(&ctx->event_cond) == 1 ||
+ kthread_should_stop()));
+
+ if (ret)
+ continue;
+
+ if (kthread_should_stop())
+ return 0;
+
+ if (!ctx->tx_skb)
+ ctx->tx_skb = cpc_interface_dequeue(ctx->intf);
+
+ /*
+ * Reset thread event right before transmission to prevent interrupts that
+ * happened while the thread was already awake to wake up the thread again,
+ * as the event is going to be handled by this iteration.
+ */
+ atomic_set(&ctx->event_cond, 0);
+
+ ret = cpc_spi_do_xfer_header(ctx);
+ if (!ret)
+ xfer_idle = false;
+ } else {
+ ret = wait_event_timeout(ctx->event_queue,
+ (atomic_read(&ctx->event_cond) == 1 ||
+ kthread_should_stop()),
+ msecs_to_jiffies(CPC_SPI_INTERRUPT_MAX_WAIT_MS));
+ if (ret == 0) {
+ dev_err_once(&ctx->spi->dev, "device didn't assert interrupt in a timely manner\n");
+ continue;
+ }
+
+ atomic_set(&ctx->event_cond, 0);
+
+ if (!ctx->tx_skb && !ctx->rx_skb)
+ ret = cpc_spi_do_xfer_notch(ctx);
+ else
+ ret = cpc_spi_do_xfer_payload(ctx);
+
+ if (!ret)
+ xfer_idle = true;
+ }
+ }
+
+ return 0;
+}
+
+static irqreturn_t cpc_spi_irq_handler(int irq, void *data)
+{
+ struct cpc_spi *ctx = data;
+
+ atomic_set(&ctx->event_cond, 1);
+ wake_up(&ctx->event_queue);
+
+ return IRQ_HANDLED;
+}
+
+static int cpc_spi_ops_wake_tx(struct cpc_interface *intf)
+{
+ struct cpc_spi *ctx = cpc_interface_get_priv(intf);
+
+ wake_up_interruptible(&ctx->event_queue);
+
+ return 0;
+}
+
+static void cpc_spi_ops_csum(struct sk_buff *skb)
+{
+ skb->csum = cpc_spi_csum(skb->data, skb->len);
+}
+
+static const struct cpc_interface_ops spi_intf_cpc_ops = {
+ .wake_tx = cpc_spi_ops_wake_tx,
+ .csum = cpc_spi_ops_csum,
+};
+
+static int cpc_spi_probe(struct spi_device *spi)
+{
+ struct cpc_interface *intf;
+ struct cpc_spi *ctx;
+ int err;
+
+ if (!spi->irq) {
+ dev_err(&spi->dev, "cannot function without IRQ, please provide one\n");
+ return -EINVAL;
+ }
+
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ intf = cpc_interface_alloc(&spi->dev, &spi_intf_cpc_ops, ctx);
+ if (IS_ERR(intf)) {
+ kfree(ctx);
+
+ return PTR_ERR(intf);
+ }
+
+ spi_set_drvdata(spi, ctx);
+
+ ctx->spi = spi;
+ ctx->intf = intf;
+
+ ctx->tx_skb = NULL;
+
+ atomic_set(&ctx->event_cond, 0);
+ ctx->rx_skb = NULL;
+
+ init_waitqueue_head(&ctx->event_queue);
+
+ err = cpc_interface_register(intf);
+ if (err)
+ goto put_interface;
+
+ err = request_irq(spi->irq, cpc_spi_irq_handler, IRQF_TRIGGER_FALLING,
+ dev_name(&spi->dev), ctx);
+ if (err)
+ goto unregister_interface;
+
+ ctx->task = kthread_run(cpc_spi_do_xfer_thread, ctx, "%s",
+ dev_name(&spi->dev));
+ if (IS_ERR(ctx->task)) {
+ err = PTR_ERR(ctx->task);
+ goto free_irq;
+ }
+
+ return 0;
+
+free_irq:
+ free_irq(spi->irq, ctx);
+
+unregister_interface:
+ cpc_interface_unregister(intf);
+
+put_interface:
+ cpc_interface_put(intf);
+
+ kfree(ctx);
+
+ return err;
+}
+
+static void cpc_spi_remove(struct spi_device *spi)
+{
+ struct cpc_spi *ctx = spi_get_drvdata(spi);
+ struct cpc_interface *intf = ctx->intf;
+
+ kthread_stop(ctx->task);
+ free_irq(spi->irq, ctx);
+ cpc_interface_unregister(intf);
+ kfree(ctx);
+}
+
+static const struct of_device_id cpc_dt_ids[] = {
+ { .compatible = "silabs,cpc-spi" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, cpc_dt_ids);
+
+static const struct spi_device_id cpc_spi_ids[] = {
+ { .name = "cpc-spi" },
+ {},
+};
+MODULE_DEVICE_TABLE(spi, cpc_spi_ids);
+
+static struct spi_driver cpc_spi_driver = {
+ .driver = {
+ .name = "cpc-spi",
+ .of_match_table = cpc_dt_ids,
+ },
+ .probe = cpc_spi_probe,
+ .remove = cpc_spi_remove,
+};
+
+/**
+ * cpc_spi_register_driver - Register driver to the SPI subsytem.
+ *
+ * @return: 0 on success, otherwise a negative error code.
+ */
+int cpc_spi_register_driver(void)
+{
+ return spi_register_driver(&cpc_spi_driver);
+}
+
+/**
+ * cpc_spi_unregister_driver - Unregister driver from the SPI subsytem.
+ */
+void cpc_spi_unregister_driver(void)
+{
+ spi_unregister_driver(&cpc_spi_driver);
+}
diff --git a/drivers/net/cpc/spi.h b/drivers/net/cpc/spi.h
new file mode 100644
index 00000000000..211133c4758
--- /dev/null
+++ b/drivers/net/cpc/spi.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_SPI_H
+#define __CPC_SPI_H
+
+int cpc_spi_register_driver(void);
+void cpc_spi_unregister_driver(void);
+
+#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC net-next 14/15] net: cpc: add SPI interface driver
2025-05-12 1:27 ` [RFC net-next 14/15] net: cpc: add SPI interface driver Damien Riégel
@ 2025-05-12 2:47 ` Andrew Lunn
0 siblings, 0 replies; 39+ messages in thread
From: Andrew Lunn @ 2025-05-12 2:47 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
On Sun, May 11, 2025 at 09:27:47PM -0400, Damien Riégel wrote:
> This adds support for CPC over SPI. CPC uses a full-duplex protocol.
> Each frame transmission/reception is split into two parts:
> - read and/or write header + header checksum
> - read and/or write payload + payload checksum
>
> Header frames are always 10 bytes (8 bytes of header and 2 bytes of
> checksum). The header contains the size of the payload to receive (size
> to transmit is already known). As the SPI device also has some
> processing to do when it receives a header, the SPI driver must wait for
> the interrupt line to be asserted before clocking the payload.
>
> The SPI device always expects the chip select to be asserted and
> deasserted after a header, even if there are no payloads to transmit.
> This is used to keep headers transmission synchronized between host and
> device. As some controllers don't support doing that if there is nothing
> to transmit, a null byte is transmitted in that case and it will be
> ignored by the device.
>
> If there are payloads, the driver will clock the maximum length of the
> two payloads.
>
> Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
> ---
> drivers/net/cpc/Kconfig | 3 +-
> drivers/net/cpc/Makefile | 2 +-
> drivers/net/cpc/main.c | 8 +
> drivers/net/cpc/spi.c | 550 +++++++++++++++++++++++++++++++++++++++
> drivers/net/cpc/spi.h | 12 +
> 5 files changed, 573 insertions(+), 2 deletions(-)
> create mode 100644 drivers/net/cpc/spi.c
> create mode 100644 drivers/net/cpc/spi.h
>
> diff --git a/drivers/net/cpc/Kconfig b/drivers/net/cpc/Kconfig
> index f31b6837b49..f5159390a82 100644
> --- a/drivers/net/cpc/Kconfig
> +++ b/drivers/net/cpc/Kconfig
> @@ -2,7 +2,8 @@
>
> menuconfig CPC
> tristate "Silicon Labs Co-Processor Communication (CPC) Protocol"
> - depends on NET
> + depends on NET && SPI
> + select CRC_ITU_T
> help
> Provide support for the CPC protocol to Silicon Labs EFR32 devices.
>
> diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
> index a61af84df90..195cdf4ad62 100644
> --- a/drivers/net/cpc/Makefile
> +++ b/drivers/net/cpc/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -cpc-y := endpoint.o header.o interface.o main.o protocol.o system.o
> +cpc-y := endpoint.o header.o interface.o main.o protocol.o spi.o system.o
>
> obj-$(CONFIG_CPC) += cpc.o
> diff --git a/drivers/net/cpc/main.c b/drivers/net/cpc/main.c
> index fc46a25f5dc..b4e73145ac2 100644
> --- a/drivers/net/cpc/main.c
> +++ b/drivers/net/cpc/main.c
> @@ -8,6 +8,7 @@
>
> #include "cpc.h"
> #include "header.h"
> +#include "spi.h"
> #include "system.h"
>
> /**
> @@ -126,12 +127,19 @@ static int __init cpc_init(void)
> if (err)
> bus_unregister(&cpc_bus);
>
> + err = cpc_spi_register_driver();
> + if (err) {
> + cpc_system_drv_unregister();
> + bus_unregister(&cpc_bus);
> + }
> +
> return err;
> }
> module_init(cpc_init);
>
> static void __exit cpc_exit(void)
> {
> + cpc_spi_unregister_driver();
> cpc_system_drv_unregister();
> bus_unregister(&cpc_bus);
> }
> diff --git a/drivers/net/cpc/spi.c b/drivers/net/cpc/spi.c
> new file mode 100644
> index 00000000000..2b068eeb5d4
> --- /dev/null
> +++ b/drivers/net/cpc/spi.c
> @@ -0,0 +1,550 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025, Silicon Laboratories, Inc.
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/crc-itu-t.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/kthread.h>
> +#include <linux/minmax.h>
> +#include <linux/of.h>
> +#include <linux/skbuff.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/unaligned.h>
> +#include <linux/wait.h>
> +
> +#include "cpc.h"
> +#include "header.h"
> +#include "interface.h"
> +#include "spi.h"
> +
> +#define CPC_SPI_CSUM_SIZE 2
> +#define CPC_SPI_INTERRUPT_MAX_WAIT_MS 1000
> +#define CPC_SPI_MAX_PAYLOAD_SIZE 4096
> +
> +struct cpc_spi {
> + struct spi_device *spi;
> + struct cpc_interface *intf;
> +
> + struct task_struct *task;
> + wait_queue_head_t event_queue;
> +
> + struct sk_buff *tx_skb;
> + u8 tx_csum[CPC_SPI_CSUM_SIZE];
> +
> + atomic_t event_cond;
> + struct sk_buff *rx_skb;
> + unsigned int rx_len;
> + u8 rx_header[CPC_HEADER_SIZE + CPC_SPI_CSUM_SIZE];
> +};
> +
> +static bool buffer_is_zeroes(const u8 *buffer, size_t length)
> +{
> + for (size_t i = 0; i < length; i++) {
> + if (buffer[i] != 0)
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static u16 cpc_spi_csum(const u8 *buffer, size_t length)
> +{
> + return crc_itu_t(0, buffer, length);
> +}
> +
> +static int cpc_spi_do_xfer_header(struct cpc_spi *ctx)
> +{
> + struct spi_transfer xfer_header = {
> + .rx_buf = ctx->rx_header,
> + .len = CPC_HEADER_SIZE,
> + .speed_hz = ctx->spi->max_speed_hz,
> + };
> + struct spi_transfer xfer_csum = {
> + .rx_buf = &ctx->rx_header[CPC_HEADER_SIZE],
> + .len = sizeof(ctx->tx_csum),
> + .speed_hz = ctx->spi->max_speed_hz,
> + };
> + enum cpc_frame_type type;
> + struct spi_message msg;
> + size_t payload_len = 0;
> + struct sk_buff *skb;
> + u16 rx_csum;
> + u16 csum;
> + int ret;
> +
> + if (ctx->tx_skb) {
> + u16 tx_hdr_csum = cpc_spi_csum(ctx->tx_skb->data, CPC_HEADER_SIZE);
> +
> + put_unaligned_le16(tx_hdr_csum, ctx->tx_csum);
> +
> + xfer_header.tx_buf = ctx->tx_skb->data;
> + xfer_csum.tx_buf = ctx->tx_csum;
> + }
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&xfer_header, &msg);
> + spi_message_add_tail(&xfer_csum, &msg);
> +
> + ret = spi_sync(ctx->spi, &msg);
> + if (ret)
> + return ret;
> +
> + if (ctx->tx_skb) {
> + if (skb_headlen(ctx->tx_skb) == CPC_HEADER_SIZE) {
> + kfree_skb(ctx->tx_skb);
> + ctx->tx_skb = NULL;
> + } else {
> + skb_pull(ctx->tx_skb, CPC_HEADER_SIZE);
> + }
> + }
> +
> + if (buffer_is_zeroes(ctx->rx_header, CPC_HEADER_SIZE))
> + return 0;
> +
> + rx_csum = get_unaligned_le16(&ctx->rx_header[CPC_HEADER_SIZE]);
> + csum = cpc_spi_csum(ctx->rx_header, CPC_HEADER_SIZE);
> +
> + if (rx_csum != csum || !cpc_header_get_type(ctx->rx_header, &type)) {
> + /*
> + * If the header checksum is invalid, its length can't be trusted, receive
> + * the maximum payload length to recover from that situation. If the frame
> + * type cannot be extracted from the header, use same recovery mechanism.
> + */
> + ctx->rx_len = CPC_SPI_MAX_PAYLOAD_SIZE;
> +
> + return 0;
> + }
> +
> + if (type == CPC_FRAME_TYPE_DATA)
> + payload_len = cpc_header_get_payload_len(ctx->rx_header) +
> + sizeof(ctx->tx_csum);
> +
> + skb = cpc_skb_alloc(payload_len, GFP_KERNEL);
> + if (!skb) {
> + /*
> + * Failed to allocate memory to receive the payload. Driver must clock in
> + * these bytes even if there is no room, to keep the sender in sync.
> + */
> + ctx->rx_len = payload_len;
> +
> + return 0;
> + }
> +
> + memcpy(skb_push(skb, CPC_HEADER_SIZE), ctx->rx_header, CPC_HEADER_SIZE);
> +
> + if (payload_len) {
> + ctx->rx_skb = skb;
> + ctx->rx_len = payload_len;
> + } else {
> + cpc_interface_receive_frame(ctx->intf, skb);
> + }
> +
> + return 0;
> +}
> +
> +static int cpc_spi_do_xfer_notch(struct cpc_spi *ctx)
> +{
> + struct spi_transfer xfer = {
> + .tx_buf = ctx->tx_csum,
> + .len = 1,
> + .speed_hz = ctx->spi->max_speed_hz,
> + };
> + struct spi_message msg;
> +
> + ctx->tx_csum[0] = 0;
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&xfer, &msg);
> +
> + return spi_sync(ctx->spi, &msg);
> +}
> +
> +static int cpc_spi_do_xfer_payload(struct cpc_spi *ctx)
> +{
> + struct spi_transfer shared_xfer = {
> + .speed_hz = ctx->spi->max_speed_hz,
> + .rx_buf = NULL,
> + .tx_buf = NULL,
> + };
> + struct spi_transfer pad_xfer1 = {
> + .speed_hz = ctx->spi->max_speed_hz,
> + .rx_buf = NULL,
> + .tx_buf = NULL,
> + };
> + struct spi_transfer pad_xfer2 = {
> + .speed_hz = ctx->spi->max_speed_hz,
> + .rx_buf = NULL,
> + .tx_buf = NULL,
> + };
> + unsigned int rx_len = ctx->rx_len;
> + unsigned int tx_data_len;
> + struct spi_message msg;
> + int ret;
> +
> + spi_message_init(&msg);
> + spi_message_add_tail(&shared_xfer, &msg);
> +
> + /*
> + * This can happen if header checksum was invalid. In that case, protocol
> + * mandates to be ready to receive the maximum number of bytes that the
> + * device is capable to send, in order to be sure its TX queue is flushed.
> + */
> + if (!ctx->rx_skb && rx_len) {
> + shared_xfer.rx_buf = kmalloc(rx_len, GFP_KERNEL);
> + if (!shared_xfer.rx_buf)
> + return -ENOMEM;
> +
> + shared_xfer.len = rx_len;
> + }
> +
> + if (ctx->rx_skb && !ctx->tx_skb) {
> + shared_xfer.rx_buf = skb_put(ctx->rx_skb, rx_len);
> + shared_xfer.len = rx_len;
> + }
> +
> + if (ctx->tx_skb) {
> + u16 csum = ctx->tx_skb->csum;
> +
> + put_unaligned_le16(csum, ctx->tx_csum);
> +
> + tx_data_len = ctx->tx_skb->len;
> +
> + shared_xfer.tx_buf = ctx->tx_skb->data;
> + shared_xfer.len = tx_data_len;
> +
> + if (!ctx->rx_skb) {
> + pad_xfer1.tx_buf = ctx->tx_csum;
> + pad_xfer1.len = sizeof(ctx->tx_csum);
> +
> + spi_message_add_tail(&pad_xfer1, &msg);
> + }
> + }
> +
> + if (ctx->rx_skb && ctx->tx_skb) {
> + unsigned int shared_len;
> + unsigned int pad_len;
> +
> + shared_len = min(rx_len, tx_data_len);
> + pad_len = max(rx_len, tx_data_len) - shared_len;
> +
> + shared_xfer.rx_buf = skb_put(ctx->rx_skb, shared_len);
> + shared_xfer.len = shared_len;
> +
> + if (rx_len < tx_data_len) {
> + /*
> + * |------- RX BUFFER + RX CSUM ------|
> + * |------------------- TX BUFFER ------------|---- TX CSUM ----|
> + *
> + * | SHARED |
> + * | PAD 1 |
> + * | PAD 2 |
> + */
> + pad_xfer1.rx_buf = NULL;
> + pad_xfer1.tx_buf = ctx->tx_skb->data + shared_len;
> + pad_xfer1.len = pad_len;
> +
> + pad_xfer2.rx_buf = NULL;
> + pad_xfer2.tx_buf = ctx->tx_csum;
> + pad_xfer2.len = sizeof(ctx->tx_csum);
> +
> + spi_message_add_tail(&pad_xfer1, &msg);
> + spi_message_add_tail(&pad_xfer2, &msg);
> + } else if (rx_len == tx_data_len) {
> + /*
> + * |------------- RX BUFFER + RX CSUM ---------|
> + * |------------------- TX BUFFER -------------|---- TX CSUM ---|
> + *
> + * | SHARED |
> + * | PAD 1 |
> + */
> + pad_xfer1.rx_buf = NULL;
> + pad_xfer1.tx_buf = ctx->tx_csum;
> + pad_xfer1.len = sizeof(ctx->tx_csum);
> +
> + spi_message_add_tail(&pad_xfer1, &msg);
> + } else if (rx_len == tx_data_len + 1) {
> + /*
> + * |----------------- RX BUFFER + RX CSUM ----------------|
> + * |------------------- TX BUFFER -------------|---- TX CSUM ---|
> + *
> + * | SHARED |
> + * | PAD 1 |
> + * | PAD 2 |
> + */
> + pad_xfer1.tx_buf = ctx->tx_csum;
> + pad_xfer1.rx_buf = skb_put(ctx->rx_skb, 1);
> + pad_xfer1.len = 1;
> +
> + pad_xfer2.tx_buf = &ctx->tx_csum[1];
> + pad_xfer2.rx_buf = NULL;
> + pad_xfer2.len = 1;
> +
> + spi_message_add_tail(&pad_xfer1, &msg);
> + spi_message_add_tail(&pad_xfer2, &msg);
> + } else {
> + /*
> + * |----------------------------- RX BUFFER + RX CSUM -------------------|
> + * |------------------- TX BUFFER -------------|---- TX CSUM ---|
> + *
> + * | SHARED |
> + * | PAD 1 |
> + * | PAD 2 |
> + */
> + pad_xfer1.tx_buf = ctx->tx_csum;
> + pad_xfer1.rx_buf = skb_put(ctx->rx_skb, sizeof(ctx->tx_csum));
> + pad_xfer1.len = sizeof(ctx->tx_csum);
> +
> + pad_xfer2.tx_buf = NULL;
> + pad_xfer2.rx_buf = skb_put(ctx->rx_skb, pad_len - sizeof(ctx->tx_csum));
> + pad_xfer2.len = pad_len - sizeof(ctx->tx_csum);
> +
> + spi_message_add_tail(&pad_xfer1, &msg);
> + spi_message_add_tail(&pad_xfer2, &msg);
> + }
> + }
> +
> + ret = spi_sync(ctx->spi, &msg);
> +
> + if (ctx->tx_skb) {
> + kfree_skb(ctx->tx_skb);
> + ctx->tx_skb = NULL;
> + }
> +
> + if (ctx->rx_skb) {
> + unsigned char *csum_ptr;
> + u16 expected_csum;
> + u16 csum;
> +
> + if (ret) {
> + kfree_skb(ctx->rx_skb);
> + goto exit;
> + }
> +
> + csum_ptr = skb_tail_pointer(ctx->rx_skb) - sizeof(csum);
> + csum = get_unaligned_le16(csum_ptr);
> +
> + expected_csum = cpc_spi_csum(ctx->rx_skb->data + CPC_HEADER_SIZE,
> + ctx->rx_len - sizeof(csum));
> +
> + if (csum == expected_csum) {
> + skb_trim(ctx->rx_skb, ctx->rx_skb->len - sizeof(csum));
> +
> + cpc_interface_receive_frame(ctx->intf, ctx->rx_skb);
> + } else {
> + kfree_skb(ctx->rx_skb);
> + }
> + }
> +
> +exit:
> + ctx->rx_skb = NULL;
> + ctx->rx_len = 0;
> +
> + return ret;
> +}
> +
> +static int cpc_spi_do_xfer_thread(void *data)
> +{
> + struct cpc_spi *ctx = data;
> + bool xfer_idle = true;
> + int ret;
> +
> + while (!kthread_should_stop()) {
> + if (xfer_idle) {
> + ret = wait_event_interruptible(ctx->event_queue,
> + (!cpc_interface_tx_queue_empty(ctx->intf) ||
> + atomic_read(&ctx->event_cond) == 1 ||
> + kthread_should_stop()));
> +
> + if (ret)
> + continue;
> +
> + if (kthread_should_stop())
> + return 0;
> +
> + if (!ctx->tx_skb)
> + ctx->tx_skb = cpc_interface_dequeue(ctx->intf);
> +
> + /*
> + * Reset thread event right before transmission to prevent interrupts that
> + * happened while the thread was already awake to wake up the thread again,
> + * as the event is going to be handled by this iteration.
> + */
> + atomic_set(&ctx->event_cond, 0);
> +
> + ret = cpc_spi_do_xfer_header(ctx);
> + if (!ret)
> + xfer_idle = false;
> + } else {
> + ret = wait_event_timeout(ctx->event_queue,
> + (atomic_read(&ctx->event_cond) == 1 ||
> + kthread_should_stop()),
> + msecs_to_jiffies(CPC_SPI_INTERRUPT_MAX_WAIT_MS));
> + if (ret == 0) {
> + dev_err_once(&ctx->spi->dev, "device didn't assert interrupt in a timely manner\n");
> + continue;
> + }
> +
> + atomic_set(&ctx->event_cond, 0);
> +
> + if (!ctx->tx_skb && !ctx->rx_skb)
> + ret = cpc_spi_do_xfer_notch(ctx);
> + else
> + ret = cpc_spi_do_xfer_payload(ctx);
> +
> + if (!ret)
> + xfer_idle = true;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static irqreturn_t cpc_spi_irq_handler(int irq, void *data)
> +{
> + struct cpc_spi *ctx = data;
> +
> + atomic_set(&ctx->event_cond, 1);
> + wake_up(&ctx->event_queue);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int cpc_spi_ops_wake_tx(struct cpc_interface *intf)
> +{
> + struct cpc_spi *ctx = cpc_interface_get_priv(intf);
> +
> + wake_up_interruptible(&ctx->event_queue);
> +
> + return 0;
> +}
> +
> +static void cpc_spi_ops_csum(struct sk_buff *skb)
> +{
> + skb->csum = cpc_spi_csum(skb->data, skb->len);
> +}
> +
> +static const struct cpc_interface_ops spi_intf_cpc_ops = {
> + .wake_tx = cpc_spi_ops_wake_tx,
> + .csum = cpc_spi_ops_csum,
> +};
> +
> +static int cpc_spi_probe(struct spi_device *spi)
> +{
> + struct cpc_interface *intf;
> + struct cpc_spi *ctx;
> + int err;
> +
> + if (!spi->irq) {
> + dev_err(&spi->dev, "cannot function without IRQ, please provide one\n");
> + return -EINVAL;
> + }
> +
> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + intf = cpc_interface_alloc(&spi->dev, &spi_intf_cpc_ops, ctx);
> + if (IS_ERR(intf)) {
> + kfree(ctx);
> +
> + return PTR_ERR(intf);
> + }
> +
> + spi_set_drvdata(spi, ctx);
> +
> + ctx->spi = spi;
> + ctx->intf = intf;
> +
> + ctx->tx_skb = NULL;
> +
> + atomic_set(&ctx->event_cond, 0);
> + ctx->rx_skb = NULL;
> +
> + init_waitqueue_head(&ctx->event_queue);
> +
> + err = cpc_interface_register(intf);
> + if (err)
> + goto put_interface;
> +
> + err = request_irq(spi->irq, cpc_spi_irq_handler, IRQF_TRIGGER_FALLING,
> + dev_name(&spi->dev), ctx);
> + if (err)
> + goto unregister_interface;
> +
> + ctx->task = kthread_run(cpc_spi_do_xfer_thread, ctx, "%s",
> + dev_name(&spi->dev));
> + if (IS_ERR(ctx->task)) {
> + err = PTR_ERR(ctx->task);
> + goto free_irq;
> + }
> +
> + return 0;
> +
> +free_irq:
> + free_irq(spi->irq, ctx);
> +
> +unregister_interface:
> + cpc_interface_unregister(intf);
> +
> +put_interface:
> + cpc_interface_put(intf);
> +
> + kfree(ctx);
> +
> + return err;
> +}
> +
> +static void cpc_spi_remove(struct spi_device *spi)
> +{
> + struct cpc_spi *ctx = spi_get_drvdata(spi);
> + struct cpc_interface *intf = ctx->intf;
> +
> + kthread_stop(ctx->task);
> + free_irq(spi->irq, ctx);
> + cpc_interface_unregister(intf);
> + kfree(ctx);
> +}
> +
> +static const struct of_device_id cpc_dt_ids[] = {
> + { .compatible = "silabs,cpc-spi" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, cpc_dt_ids);
> +
> +static const struct spi_device_id cpc_spi_ids[] = {
> + { .name = "cpc-spi" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(spi, cpc_spi_ids);
> +
> +static struct spi_driver cpc_spi_driver = {
> + .driver = {
> + .name = "cpc-spi",
> + .of_match_table = cpc_dt_ids,
> + },
Quoting an earlier patch:
> As a very basic matching mechanism, the bus will match an endpoint
> with its driver if driver's name (driver.name attribute) matches
> endpoint's name.
Don't you want silabs in the name, so you can tell it from some other
vendors SPI bus?
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC net-next 15/15] net: cpc: add Bluetooth HCI driver
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (13 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 14/15] net: cpc: add SPI interface driver Damien Riégel
@ 2025-05-12 1:27 ` Damien Riégel
2025-05-12 17:07 ` [RFC net-next 00/15] Add support for Silicon Labs CPC Andrew Lunn
15 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-12 1:27 UTC (permalink / raw)
To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel
Add support for Bluetooth HCI driver. As most of the protocol is already
handled by the remote endpoint, this driver is just doing some glue to
plug into CPC.
Signed-off-by: Damien Riégel <damien.riegel@silabs.com>
---
drivers/net/cpc/Kconfig | 2 +-
drivers/net/cpc/Makefile | 2 +-
drivers/net/cpc/ble.c | 147 +++++++++++++++++++++++++++++++++++++++
drivers/net/cpc/ble.h | 14 ++++
drivers/net/cpc/main.c | 23 ++++--
5 files changed, 181 insertions(+), 7 deletions(-)
create mode 100644 drivers/net/cpc/ble.c
create mode 100644 drivers/net/cpc/ble.h
diff --git a/drivers/net/cpc/Kconfig b/drivers/net/cpc/Kconfig
index f5159390a82..e8faa351bf7 100644
--- a/drivers/net/cpc/Kconfig
+++ b/drivers/net/cpc/Kconfig
@@ -2,7 +2,7 @@
menuconfig CPC
tristate "Silicon Labs Co-Processor Communication (CPC) Protocol"
- depends on NET && SPI
+ depends on NET && SPI && BT
select CRC_ITU_T
help
Provide support for the CPC protocol to Silicon Labs EFR32 devices.
diff --git a/drivers/net/cpc/Makefile b/drivers/net/cpc/Makefile
index 195cdf4ad62..cee40aec412 100644
--- a/drivers/net/cpc/Makefile
+++ b/drivers/net/cpc/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-cpc-y := endpoint.o header.o interface.o main.o protocol.o spi.o system.o
+cpc-y := ble.o endpoint.o header.o interface.o main.o protocol.o spi.o system.o
obj-$(CONFIG_CPC) += cpc.o
diff --git a/drivers/net/cpc/ble.c b/drivers/net/cpc/ble.c
new file mode 100644
index 00000000000..2b7aec4dbdf
--- /dev/null
+++ b/drivers/net/cpc/ble.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Bluetooth HCI over CPC.
+ *
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#include <linux/skbuff.h>
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+
+#include "ble.h"
+#include "cpc.h"
+
+struct cpc_ble {
+ struct cpc_endpoint *ep;
+ struct hci_dev *hdev;
+ struct sk_buff_head txq;
+};
+
+static int cpc_ble_open(struct hci_dev *hdev)
+{
+ struct cpc_ble *ble = hci_get_drvdata(hdev);
+
+ skb_queue_head_init(&ble->txq);
+
+ return cpc_endpoint_connect(ble->ep);
+}
+
+static int cpc_ble_close(struct hci_dev *hdev)
+{
+ struct cpc_ble *ble = hci_get_drvdata(hdev);
+
+ cpc_endpoint_disconnect(ble->ep);
+
+ skb_queue_purge(&ble->txq);
+
+ return 0;
+}
+
+static int cpc_ble_flush(struct hci_dev *hdev)
+{
+ struct cpc_ble *ble = hci_get_drvdata(hdev);
+
+ skb_queue_purge(&ble->txq);
+
+ return 0;
+}
+
+static int cpc_ble_send(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct cpc_ble *ble = hci_get_drvdata(hdev);
+
+ memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
+
+ return cpc_endpoint_write(ble->ep, skb);
+}
+
+static void cpc_ble_rx_frame(struct cpc_endpoint *ep, struct sk_buff *skb)
+{
+ struct cpc_ble *ble = cpc_endpoint_get_drvdata(ep);
+
+ hci_skb_pkt_type(skb) = *((u8 *)skb_pull_data(skb, 1));
+ hci_skb_expect(skb) = skb->len;
+
+ hci_recv_frame(ble->hdev, skb);
+}
+
+static struct cpc_endpoint_ops cpc_ble_ops = {
+ .rx = cpc_ble_rx_frame,
+};
+
+static int cpc_ble_probe(struct cpc_endpoint *ep)
+{
+ struct cpc_ble *ble;
+ int err;
+
+ ble = kzalloc(sizeof(*ble), GFP_KERNEL);
+ if (!ble) {
+ err = -ENOMEM;
+ goto alloc_ble_fail;
+ }
+
+ cpc_endpoint_set_ops(ep, &cpc_ble_ops);
+ cpc_endpoint_set_drvdata(ep, ble);
+
+ ble->ep = ep;
+ ble->hdev = hci_alloc_dev();
+ if (!ble->hdev) {
+ err = -ENOMEM;
+ goto alloc_hdev_fail;
+ }
+
+ hci_set_drvdata(ble->hdev, ble);
+ ble->hdev->open = cpc_ble_open;
+ ble->hdev->close = cpc_ble_close;
+ ble->hdev->flush = cpc_ble_flush;
+ ble->hdev->send = cpc_ble_send;
+
+ err = hci_register_dev(ble->hdev);
+ if (err)
+ goto register_hdev_fail;
+
+ return 0;
+
+register_hdev_fail:
+ hci_free_dev(ble->hdev);
+alloc_hdev_fail:
+ kfree(ble);
+alloc_ble_fail:
+ return err;
+}
+
+static void cpc_ble_remove(struct cpc_endpoint *ep)
+{
+ struct cpc_ble *ble = cpc_endpoint_get_drvdata(ep);
+
+ hci_unregister_dev(ble->hdev);
+ hci_free_dev(ble->hdev);
+ kfree(ble);
+}
+
+static struct cpc_driver ble_driver = {
+ .driver = {
+ .name = CPC_BLUETOOTH_ENDPOINT_NAME,
+ },
+ .probe = cpc_ble_probe,
+ .remove = cpc_ble_remove,
+};
+
+/**
+ * cpc_ble_drv_register - Register the ble endpoint driver.
+ *
+ * @return: 0 on success, otherwise a negative error code.
+ */
+int cpc_ble_drv_register(void)
+{
+ return cpc_driver_register(&ble_driver);
+}
+
+/**
+ * cpc_ble_drv_unregister - Unregister the ble endpoint driver.
+ */
+void cpc_ble_drv_unregister(void)
+{
+ cpc_driver_unregister(&ble_driver);
+}
diff --git a/drivers/net/cpc/ble.h b/drivers/net/cpc/ble.h
new file mode 100644
index 00000000000..ae1cac4e7e8
--- /dev/null
+++ b/drivers/net/cpc/ble.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2025, Silicon Laboratories, Inc.
+ */
+
+#ifndef __CPC_BLE_H
+#define __CPC_BLE_H
+
+#define CPC_BLUETOOTH_ENDPOINT_NAME "silabs,cpc-ble"
+
+int cpc_ble_drv_register(void);
+void cpc_ble_drv_unregister(void);
+
+#endif
diff --git a/drivers/net/cpc/main.c b/drivers/net/cpc/main.c
index b4e73145ac2..e5636207d5d 100644
--- a/drivers/net/cpc/main.c
+++ b/drivers/net/cpc/main.c
@@ -7,6 +7,7 @@
#include <linux/module.h>
#include "cpc.h"
+#include "ble.h"
#include "header.h"
#include "spi.h"
#include "system.h"
@@ -125,13 +126,24 @@ static int __init cpc_init(void)
err = cpc_system_drv_register();
if (err)
- bus_unregister(&cpc_bus);
+ goto unregister_bus;
+
+ err = cpc_ble_drv_register();
+ if (err)
+ goto unregister_system_driver;
err = cpc_spi_register_driver();
- if (err) {
- cpc_system_drv_unregister();
- bus_unregister(&cpc_bus);
- }
+ if (err)
+ goto unregister_ble_driver;
+
+ return 0;
+
+unregister_ble_driver:
+ cpc_ble_drv_unregister();
+unregister_system_driver:
+ cpc_system_drv_unregister();
+unregister_bus:
+ bus_unregister(&cpc_bus);
return err;
}
@@ -140,6 +152,7 @@ module_init(cpc_init);
static void __exit cpc_exit(void)
{
cpc_spi_unregister_driver();
+ cpc_ble_drv_unregister();
cpc_system_drv_unregister();
bus_unregister(&cpc_bus);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-12 1:27 [RFC net-next 00/15] Add support for Silicon Labs CPC Damien Riégel
` (14 preceding siblings ...)
2025-05-12 1:27 ` [RFC net-next 15/15] net: cpc: add Bluetooth HCI driver Damien Riégel
@ 2025-05-12 17:07 ` Andrew Lunn
2025-05-13 21:15 ` Damien Riégel
15 siblings, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-05-12 17:07 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, greybus-dev
On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
> Hi,
>
>
> This patchset brings initial support for Silicon Labs CPC protocol,
> standing for Co-Processor Communication. This protocol is used by the
> EFR32 Series [1]. These devices offer a variety for radio protocols,
> such as Bluetooth, Z-Wave, Zigbee [2].
Before we get too deep into the details of the patches, please could
you do a compare/contrast to Greybus.
The core of Greybus is already in the kernel, with some more bits
being in staging. I don't know it too well, but at first glance it
looks very similar. We should not duplicate that.
Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
has support for these, although the code is current in staging. But
for staging code, it is actually pretty good.
Why should we add a vendor implementation when we already appear to
have something which does most of what is needed?
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-12 17:07 ` [RFC net-next 00/15] Add support for Silicon Labs CPC Andrew Lunn
@ 2025-05-13 21:15 ` Damien Riégel
2025-05-13 21:53 ` Andrew Lunn
0 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-13 21:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, greybus-dev
On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
> On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
>> Hi,
>>
>>
>> This patchset brings initial support for Silicon Labs CPC protocol,
>> standing for Co-Processor Communication. This protocol is used by the
>> EFR32 Series [1]. These devices offer a variety for radio protocols,
>> such as Bluetooth, Z-Wave, Zigbee [2].
>
> Before we get too deep into the details of the patches, please could
> you do a compare/contrast to Greybus.
Thank you for the prompt feedback on the RFC. We took a look at Greybus
in the past and it didn't seem to fit our needs. One of the main use
case that drove the development of CPC was to support WiFi (in
coexistence with other radio stacks) over SDIO, and get the maximum
throughput possible. We concluded that to achieve this we would need
packet aggregation, as sending one frame at a time over SDIO is
wasteful, and managing Radio Co-Processor available buffers, as sending
frames that the RCP is not able to process would degrade performance.
Greybus don't seem to offer these capabilities. It seems to be more
geared towards implementing RPC, where the host would send a command,
and then wait for the device to execute it and to respond. For Greybus'
protocols that implement some "streaming" features like audio or video
capture, the data streams go to an I2S or CSI interface, but it doesn't
seem to go through a CPort. So it seems to act as a backbone to connect
CPorts together, but high-throughput transfers happen on other types of
links. CPC is more about moving data over a physical link, guaranteeing
ordered delivery and avoiding unnecessary transmissions if remote
doesn't have the resources, it's much lower level than Greybus.
> Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
> the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
> has support for these, although the code is current in staging. But
> for staging code, it is actually pretty good.
I agree with you that the EFR32 is a general purpose SoC and exposing
all available peripherals would be great, but most customers buy it as
an RCP module with one or more radio stacks enabled, and that's the
situation we're trying to address. Maybe I introduced a framework with
custom bus, drivers and endpoints where it was unnecessary, the goal is
not to be super generic but only to support coexistence of our radio
stacks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-13 21:15 ` Damien Riégel
@ 2025-05-13 21:53 ` Andrew Lunn
2025-05-14 22:52 ` Damien Riégel
0 siblings, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-05-13 21:53 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, greybus-dev
On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
> > On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
> >> Hi,
> >>
> >>
> >> This patchset brings initial support for Silicon Labs CPC protocol,
> >> standing for Co-Processor Communication. This protocol is used by the
> >> EFR32 Series [1]. These devices offer a variety for radio protocols,
> >> such as Bluetooth, Z-Wave, Zigbee [2].
> >
> > Before we get too deep into the details of the patches, please could
> > you do a compare/contrast to Greybus.
>
> Thank you for the prompt feedback on the RFC. We took a look at Greybus
> in the past and it didn't seem to fit our needs. One of the main use
> case that drove the development of CPC was to support WiFi (in
> coexistence with other radio stacks) over SDIO, and get the maximum
> throughput possible. We concluded that to achieve this we would need
> packet aggregation, as sending one frame at a time over SDIO is
> wasteful, and managing Radio Co-Processor available buffers, as sending
> frames that the RCP is not able to process would degrade performance.
>
> Greybus don't seem to offer these capabilities. It seems to be more
> geared towards implementing RPC, where the host would send a command,
> and then wait for the device to execute it and to respond. For Greybus'
> protocols that implement some "streaming" features like audio or video
> capture, the data streams go to an I2S or CSI interface, but it doesn't
> seem to go through a CPort. So it seems to act as a backbone to connect
> CPorts together, but high-throughput transfers happen on other types of
> links. CPC is more about moving data over a physical link, guaranteeing
> ordered delivery and avoiding unnecessary transmissions if remote
> doesn't have the resources, it's much lower level than Greybus.
As is said, i don't know Greybus too well. I hope its Maintainers can
comment on this.
> > Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
> > the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
> > has support for these, although the code is current in staging. But
> > for staging code, it is actually pretty good.
>
> I agree with you that the EFR32 is a general purpose SoC and exposing
> all available peripherals would be great, but most customers buy it as
> an RCP module with one or more radio stacks enabled, and that's the
> situation we're trying to address. Maybe I introduced a framework with
> custom bus, drivers and endpoints where it was unnecessary, the goal is
> not to be super generic but only to support coexistence of our radio
> stacks.
This leads to my next problem.
https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
Nordic Semiconductor has what appears to be a similar device.
https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
Microchip has a similar device as well.
https://www.ti.com/product/CC2674R10
TI has a similar device.
And maybe there are others?
Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
CPC, and an ACME CPC?
How do we end up with one implementation?
Maybe Greybus does not currently support your streaming use case too
well, but it is at least vendor neutral. Can it be extended for
streaming?
And maybe a dumb question... How do transfers get out of order over
SPI and SDIO? If you look at the Open Alliance TC6 specification for
Ethernet over SPI, it does not have any issues with ordering.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-13 21:53 ` Andrew Lunn
@ 2025-05-14 22:52 ` Damien Riégel
2025-05-15 7:49 ` Greg Kroah-Hartman
2025-05-22 2:46 ` Alex Elder
0 siblings, 2 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-14 22:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, greybus-dev
On Tue May 13, 2025 at 5:53 PM EDT, Andrew Lunn wrote:
> On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
>> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
>> > On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
>> >> Hi,
>> >>
>> >>
>> >> This patchset brings initial support for Silicon Labs CPC protocol,
>> >> standing for Co-Processor Communication. This protocol is used by the
>> >> EFR32 Series [1]. These devices offer a variety for radio protocols,
>> >> such as Bluetooth, Z-Wave, Zigbee [2].
>> >
>> > Before we get too deep into the details of the patches, please could
>> > you do a compare/contrast to Greybus.
>>
>> Thank you for the prompt feedback on the RFC. We took a look at Greybus
>> in the past and it didn't seem to fit our needs. One of the main use
>> case that drove the development of CPC was to support WiFi (in
>> coexistence with other radio stacks) over SDIO, and get the maximum
>> throughput possible. We concluded that to achieve this we would need
>> packet aggregation, as sending one frame at a time over SDIO is
>> wasteful, and managing Radio Co-Processor available buffers, as sending
>> frames that the RCP is not able to process would degrade performance.
>>
>> Greybus don't seem to offer these capabilities. It seems to be more
>> geared towards implementing RPC, where the host would send a command,
>> and then wait for the device to execute it and to respond. For Greybus'
>> protocols that implement some "streaming" features like audio or video
>> capture, the data streams go to an I2S or CSI interface, but it doesn't
>> seem to go through a CPort. So it seems to act as a backbone to connect
>> CPorts together, but high-throughput transfers happen on other types of
>> links. CPC is more about moving data over a physical link, guaranteeing
>> ordered delivery and avoiding unnecessary transmissions if remote
>> doesn't have the resources, it's much lower level than Greybus.
>
> As is said, i don't know Greybus too well. I hope its Maintainers can
> comment on this.
>
>> > Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
>> > the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
>> > has support for these, although the code is current in staging. But
>> > for staging code, it is actually pretty good.
>>
>> I agree with you that the EFR32 is a general purpose SoC and exposing
>> all available peripherals would be great, but most customers buy it as
>> an RCP module with one or more radio stacks enabled, and that's the
>> situation we're trying to address. Maybe I introduced a framework with
>> custom bus, drivers and endpoints where it was unnecessary, the goal is
>> not to be super generic but only to support coexistence of our radio
>> stacks.
>
> This leads to my next problem.
>
> https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
> Nordic Semiconductor has what appears to be a similar device.
>
> https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
> Microchip has a similar device as well.
>
> https://www.ti.com/product/CC2674R10
> TI has a similar device.
>
> And maybe there are others?
>
> Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
> CPC, and an ACME CPC?
>
> How do we end up with one implementation?
>
> Maybe Greybus does not currently support your streaming use case too
> well, but it is at least vendor neutral. Can it be extended for
> streaming?
I get the sentiment that we don't want every single vendor to push their
own protocols that are ever so slightly different. To be honest, I don't
know if Greybus can be extended for that use case, or if it's something
they are interested in supporting. I've subscribed to greybus-dev so
hopefully my email will get through this time (previous one is pending
approval).
Unfortunately, we're deep down the CPC road, especially on the firmware
side. Blame on me for not sending the RFC sooner and getting feedback
earlier, but if we have to massively change our course of action we need
some degree of confidence that this is a viable alternative for
achieving high-throughput for WiFi over SDIO. I would really value any
input from the Greybus folks on this.
> And maybe a dumb question... How do transfers get out of order over
> SPI and SDIO? If you look at the Open Alliance TC6 specification for
> Ethernet over SPI, it does not have any issues with ordering.
Sorry I wasn't very clear about that. Of course packets are sent in
order but several packets can be sent at once before being acknowledged
and we might detect CRC errors on one of these packets. CPC takes care
of only delivering valid packets, and packets that come after the one
with CRC error won't be delivered to upper layer until the faulty one is
retransmitted.
I took a look at the specification you mentioned and they completely
delegate that to upper layers:
When transmit or receive frame bit errors are detected on the SPI,
the retry of frames is performed by higher protocol layers that are
beyond the scope of this specification. [1]
Our goal was to be agnostic of stacks on top of CPC and reliably
transmit frames. To give a bit of context, CPC was originally derived
from HDLC, which features detecting sequence gaps and retransmission. On
top of that, we've now added the mechanism I mentioned in previous
emails that throttle the host when the RCP is not ready to receive and
process frames on an endpoint.
[1] https://opensig.org/wp-content/uploads/2023/12/OPEN_Alliance_10BASET1x_MAC-PHY_Serial_Interface_V1.1.pdf (Section 7.3.1)
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-14 22:52 ` Damien Riégel
@ 2025-05-15 7:49 ` Greg Kroah-Hartman
2025-05-15 15:00 ` Damien Riégel
2025-05-22 2:46 ` Alex Elder
1 sibling, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2025-05-15 7:49 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On Wed, May 14, 2025 at 06:52:27PM -0400, Damien Riégel wrote:
> On Tue May 13, 2025 at 5:53 PM EDT, Andrew Lunn wrote:
> > On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
> >> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
> >> > On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
> >> >> Hi,
> >> >>
> >> >>
> >> >> This patchset brings initial support for Silicon Labs CPC protocol,
> >> >> standing for Co-Processor Communication. This protocol is used by the
> >> >> EFR32 Series [1]. These devices offer a variety for radio protocols,
> >> >> such as Bluetooth, Z-Wave, Zigbee [2].
> >> >
> >> > Before we get too deep into the details of the patches, please could
> >> > you do a compare/contrast to Greybus.
> >>
> >> Thank you for the prompt feedback on the RFC. We took a look at Greybus
> >> in the past and it didn't seem to fit our needs. One of the main use
> >> case that drove the development of CPC was to support WiFi (in
> >> coexistence with other radio stacks) over SDIO, and get the maximum
> >> throughput possible. We concluded that to achieve this we would need
> >> packet aggregation, as sending one frame at a time over SDIO is
> >> wasteful, and managing Radio Co-Processor available buffers, as sending
> >> frames that the RCP is not able to process would degrade performance.
> >>
> >> Greybus don't seem to offer these capabilities. It seems to be more
> >> geared towards implementing RPC, where the host would send a command,
> >> and then wait for the device to execute it and to respond. For Greybus'
> >> protocols that implement some "streaming" features like audio or video
> >> capture, the data streams go to an I2S or CSI interface, but it doesn't
> >> seem to go through a CPort. So it seems to act as a backbone to connect
> >> CPorts together, but high-throughput transfers happen on other types of
> >> links. CPC is more about moving data over a physical link, guaranteeing
> >> ordered delivery and avoiding unnecessary transmissions if remote
> >> doesn't have the resources, it's much lower level than Greybus.
> >
> > As is said, i don't know Greybus too well. I hope its Maintainers can
> > comment on this.
> >
> >> > Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
> >> > the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
> >> > has support for these, although the code is current in staging. But
> >> > for staging code, it is actually pretty good.
> >>
> >> I agree with you that the EFR32 is a general purpose SoC and exposing
> >> all available peripherals would be great, but most customers buy it as
> >> an RCP module with one or more radio stacks enabled, and that's the
> >> situation we're trying to address. Maybe I introduced a framework with
> >> custom bus, drivers and endpoints where it was unnecessary, the goal is
> >> not to be super generic but only to support coexistence of our radio
> >> stacks.
> >
> > This leads to my next problem.
> >
> > https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
> > Nordic Semiconductor has what appears to be a similar device.
> >
> > https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
> > Microchip has a similar device as well.
> >
> > https://www.ti.com/product/CC2674R10
> > TI has a similar device.
> >
> > And maybe there are others?
> >
> > Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
> > CPC, and an ACME CPC?
> >
> > How do we end up with one implementation?
> >
> > Maybe Greybus does not currently support your streaming use case too
> > well, but it is at least vendor neutral. Can it be extended for
> > streaming?
>
> I get the sentiment that we don't want every single vendor to push their
> own protocols that are ever so slightly different. To be honest, I don't
> know if Greybus can be extended for that use case, or if it's something
> they are interested in supporting. I've subscribed to greybus-dev so
> hopefully my email will get through this time (previous one is pending
> approval).
>
> Unfortunately, we're deep down the CPC road, especially on the firmware
> side. Blame on me for not sending the RFC sooner and getting feedback
> earlier, but if we have to massively change our course of action we need
> some degree of confidence that this is a viable alternative for
> achieving high-throughput for WiFi over SDIO. I would really value any
> input from the Greybus folks on this.
So what you are looking for is a standard way to "tunnel" SDIO over some
other physical transport, right? If so, then yes, please use Greybus as
that is exactly what it was designed for.
If there is a throughput issue with the sdio implementation on Greybus,
we can address it by fixing up the code to go faster, I don't recall
there ever being any real benchmarking happening for that protocol in
the past as the physical layer that we were using for Greybus at the
time (MIPI) was very fast, the bottleneck was usually either the host
controller we were using for Greybus, OR on the firmware side in the
device itself (i.e. turning Greybus packets into SDIO commands, as SDIO
was pretty slow.)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-15 7:49 ` Greg Kroah-Hartman
@ 2025-05-15 15:00 ` Damien Riégel
2025-05-16 7:51 ` Greg Kroah-Hartman
0 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-15 15:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andrew Lunn, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On Thu May 15, 2025 at 3:49 AM EDT, Greg Kroah-Hartman wrote:
> On Wed, May 14, 2025 at 06:52:27PM -0400, Damien Riégel wrote:
>> On Tue May 13, 2025 at 5:53 PM EDT, Andrew Lunn wrote:
>> > On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
>> >> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
>> >> > On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
>> >> >> Hi,
>> >> >>
>> >> >>
>> >> >> This patchset brings initial support for Silicon Labs CPC protocol,
>> >> >> standing for Co-Processor Communication. This protocol is used by the
>> >> >> EFR32 Series [1]. These devices offer a variety for radio protocols,
>> >> >> such as Bluetooth, Z-Wave, Zigbee [2].
>> >> >
>> >> > Before we get too deep into the details of the patches, please could
>> >> > you do a compare/contrast to Greybus.
>> >>
>> >> Thank you for the prompt feedback on the RFC. We took a look at Greybus
>> >> in the past and it didn't seem to fit our needs. One of the main use
>> >> case that drove the development of CPC was to support WiFi (in
>> >> coexistence with other radio stacks) over SDIO, and get the maximum
>> >> throughput possible. We concluded that to achieve this we would need
>> >> packet aggregation, as sending one frame at a time over SDIO is
>> >> wasteful, and managing Radio Co-Processor available buffers, as sending
>> >> frames that the RCP is not able to process would degrade performance.
>> >>
>> >> Greybus don't seem to offer these capabilities. It seems to be more
>> >> geared towards implementing RPC, where the host would send a command,
>> >> and then wait for the device to execute it and to respond. For Greybus'
>> >> protocols that implement some "streaming" features like audio or video
>> >> capture, the data streams go to an I2S or CSI interface, but it doesn't
>> >> seem to go through a CPort. So it seems to act as a backbone to connect
>> >> CPorts together, but high-throughput transfers happen on other types of
>> >> links. CPC is more about moving data over a physical link, guaranteeing
>> >> ordered delivery and avoiding unnecessary transmissions if remote
>> >> doesn't have the resources, it's much lower level than Greybus.
>> >
>> > As is said, i don't know Greybus too well. I hope its Maintainers can
>> > comment on this.
>> >
>> >> > Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
>> >> > the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
>> >> > has support for these, although the code is current in staging. But
>> >> > for staging code, it is actually pretty good.
>> >>
>> >> I agree with you that the EFR32 is a general purpose SoC and exposing
>> >> all available peripherals would be great, but most customers buy it as
>> >> an RCP module with one or more radio stacks enabled, and that's the
>> >> situation we're trying to address. Maybe I introduced a framework with
>> >> custom bus, drivers and endpoints where it was unnecessary, the goal is
>> >> not to be super generic but only to support coexistence of our radio
>> >> stacks.
>> >
>> > This leads to my next problem.
>> >
>> > https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
>> > Nordic Semiconductor has what appears to be a similar device.
>> >
>> > https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
>> > Microchip has a similar device as well.
>> >
>> > https://www.ti.com/product/CC2674R10
>> > TI has a similar device.
>> >
>> > And maybe there are others?
>> >
>> > Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
>> > CPC, and an ACME CPC?
>> >
>> > How do we end up with one implementation?
>> >
>> > Maybe Greybus does not currently support your streaming use case too
>> > well, but it is at least vendor neutral. Can it be extended for
>> > streaming?
>>
>> I get the sentiment that we don't want every single vendor to push their
>> own protocols that are ever so slightly different. To be honest, I don't
>> know if Greybus can be extended for that use case, or if it's something
>> they are interested in supporting. I've subscribed to greybus-dev so
>> hopefully my email will get through this time (previous one is pending
>> approval).
>>
>> Unfortunately, we're deep down the CPC road, especially on the firmware
>> side. Blame on me for not sending the RFC sooner and getting feedback
>> earlier, but if we have to massively change our course of action we need
>> some degree of confidence that this is a viable alternative for
>> achieving high-throughput for WiFi over SDIO. I would really value any
>> input from the Greybus folks on this.
>
> So what you are looking for is a standard way to "tunnel" SDIO over some
> other physical transport, right? If so, then yes, please use Greybus as
> that is exactly what it was designed for.
No, we want to use SDIO as physical transport. To use the Greybus
terminology, our MCUs would act as modules with a single interface, and
that interface would have "radio" bundles for each of the supported
stack.
So we want to expose our radio stacks in Linux and Greybus doesn't
define protocols for that, so that's kind of uncharted territories and
we were wondering if Greybus would be the right tool for that. I hope
the situation is a bit clearer now.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-15 15:00 ` Damien Riégel
@ 2025-05-16 7:51 ` Greg Kroah-Hartman
2025-05-16 16:25 ` Damien Riégel
0 siblings, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2025-05-16 7:51 UTC (permalink / raw)
To: Damien Riégel
Cc: Andrew Lunn, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On Thu, May 15, 2025 at 11:00:39AM -0400, Damien Riégel wrote:
> On Thu May 15, 2025 at 3:49 AM EDT, Greg Kroah-Hartman wrote:
> > On Wed, May 14, 2025 at 06:52:27PM -0400, Damien Riégel wrote:
> >> On Tue May 13, 2025 at 5:53 PM EDT, Andrew Lunn wrote:
> >> > On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
> >> >> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
> >> >> > On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
> >> >> >> Hi,
> >> >> >>
> >> >> >>
> >> >> >> This patchset brings initial support for Silicon Labs CPC protocol,
> >> >> >> standing for Co-Processor Communication. This protocol is used by the
> >> >> >> EFR32 Series [1]. These devices offer a variety for radio protocols,
> >> >> >> such as Bluetooth, Z-Wave, Zigbee [2].
> >> >> >
> >> >> > Before we get too deep into the details of the patches, please could
> >> >> > you do a compare/contrast to Greybus.
> >> >>
> >> >> Thank you for the prompt feedback on the RFC. We took a look at Greybus
> >> >> in the past and it didn't seem to fit our needs. One of the main use
> >> >> case that drove the development of CPC was to support WiFi (in
> >> >> coexistence with other radio stacks) over SDIO, and get the maximum
> >> >> throughput possible. We concluded that to achieve this we would need
> >> >> packet aggregation, as sending one frame at a time over SDIO is
> >> >> wasteful, and managing Radio Co-Processor available buffers, as sending
> >> >> frames that the RCP is not able to process would degrade performance.
> >> >>
> >> >> Greybus don't seem to offer these capabilities. It seems to be more
> >> >> geared towards implementing RPC, where the host would send a command,
> >> >> and then wait for the device to execute it and to respond. For Greybus'
> >> >> protocols that implement some "streaming" features like audio or video
> >> >> capture, the data streams go to an I2S or CSI interface, but it doesn't
> >> >> seem to go through a CPort. So it seems to act as a backbone to connect
> >> >> CPorts together, but high-throughput transfers happen on other types of
> >> >> links. CPC is more about moving data over a physical link, guaranteeing
> >> >> ordered delivery and avoiding unnecessary transmissions if remote
> >> >> doesn't have the resources, it's much lower level than Greybus.
> >> >
> >> > As is said, i don't know Greybus too well. I hope its Maintainers can
> >> > comment on this.
> >> >
> >> >> > Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
> >> >> > the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
> >> >> > has support for these, although the code is current in staging. But
> >> >> > for staging code, it is actually pretty good.
> >> >>
> >> >> I agree with you that the EFR32 is a general purpose SoC and exposing
> >> >> all available peripherals would be great, but most customers buy it as
> >> >> an RCP module with one or more radio stacks enabled, and that's the
> >> >> situation we're trying to address. Maybe I introduced a framework with
> >> >> custom bus, drivers and endpoints where it was unnecessary, the goal is
> >> >> not to be super generic but only to support coexistence of our radio
> >> >> stacks.
> >> >
> >> > This leads to my next problem.
> >> >
> >> > https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
> >> > Nordic Semiconductor has what appears to be a similar device.
> >> >
> >> > https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
> >> > Microchip has a similar device as well.
> >> >
> >> > https://www.ti.com/product/CC2674R10
> >> > TI has a similar device.
> >> >
> >> > And maybe there are others?
> >> >
> >> > Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
> >> > CPC, and an ACME CPC?
> >> >
> >> > How do we end up with one implementation?
> >> >
> >> > Maybe Greybus does not currently support your streaming use case too
> >> > well, but it is at least vendor neutral. Can it be extended for
> >> > streaming?
> >>
> >> I get the sentiment that we don't want every single vendor to push their
> >> own protocols that are ever so slightly different. To be honest, I don't
> >> know if Greybus can be extended for that use case, or if it's something
> >> they are interested in supporting. I've subscribed to greybus-dev so
> >> hopefully my email will get through this time (previous one is pending
> >> approval).
> >>
> >> Unfortunately, we're deep down the CPC road, especially on the firmware
> >> side. Blame on me for not sending the RFC sooner and getting feedback
> >> earlier, but if we have to massively change our course of action we need
> >> some degree of confidence that this is a viable alternative for
> >> achieving high-throughput for WiFi over SDIO. I would really value any
> >> input from the Greybus folks on this.
> >
> > So what you are looking for is a standard way to "tunnel" SDIO over some
> > other physical transport, right? If so, then yes, please use Greybus as
> > that is exactly what it was designed for.
>
> No, we want to use SDIO as physical transport. To use the Greybus
> terminology, our MCUs would act as modules with a single interface, and
> that interface would have "radio" bundles for each of the supported
> stack.
>
> So we want to expose our radio stacks in Linux and Greybus doesn't
> define protocols for that, so that's kind of uncharted territories and
> we were wondering if Greybus would be the right tool for that. I hope
> the situation is a bit clearer now.
Yes, greybus does not expose a "wifi" protocol as that is way too device
specific, sorry.
So this just would be like any other normal SDIO wifi device then,
shouldn't be anything special, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-16 7:51 ` Greg Kroah-Hartman
@ 2025-05-16 16:25 ` Damien Riégel
2025-05-18 15:23 ` Andrew Lunn
0 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-16 16:25 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andrew Lunn, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On Fri May 16, 2025 at 3:51 AM EDT, Greg Kroah-Hartman wrote:
> On Thu, May 15, 2025 at 11:00:39AM -0400, Damien Riégel wrote:
>> On Thu May 15, 2025 at 3:49 AM EDT, Greg Kroah-Hartman wrote:
>> > On Wed, May 14, 2025 at 06:52:27PM -0400, Damien Riégel wrote:
>> >> On Tue May 13, 2025 at 5:53 PM EDT, Andrew Lunn wrote:
>> >> > On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
>> >> >> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
>> >> >> > On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
>> >> >> >> Hi,
>> >> >> >>
>> >> >> >>
>> >> >> >> This patchset brings initial support for Silicon Labs CPC protocol,
>> >> >> >> standing for Co-Processor Communication. This protocol is used by the
>> >> >> >> EFR32 Series [1]. These devices offer a variety for radio protocols,
>> >> >> >> such as Bluetooth, Z-Wave, Zigbee [2].
>> >> >> >
>> >> >> > Before we get too deep into the details of the patches, please could
>> >> >> > you do a compare/contrast to Greybus.
>> >> >>
>> >> >> Thank you for the prompt feedback on the RFC. We took a look at Greybus
>> >> >> in the past and it didn't seem to fit our needs. One of the main use
>> >> >> case that drove the development of CPC was to support WiFi (in
>> >> >> coexistence with other radio stacks) over SDIO, and get the maximum
>> >> >> throughput possible. We concluded that to achieve this we would need
>> >> >> packet aggregation, as sending one frame at a time over SDIO is
>> >> >> wasteful, and managing Radio Co-Processor available buffers, as sending
>> >> >> frames that the RCP is not able to process would degrade performance.
>> >> >>
>> >> >> Greybus don't seem to offer these capabilities. It seems to be more
>> >> >> geared towards implementing RPC, where the host would send a command,
>> >> >> and then wait for the device to execute it and to respond. For Greybus'
>> >> >> protocols that implement some "streaming" features like audio or video
>> >> >> capture, the data streams go to an I2S or CSI interface, but it doesn't
>> >> >> seem to go through a CPort. So it seems to act as a backbone to connect
>> >> >> CPorts together, but high-throughput transfers happen on other types of
>> >> >> links. CPC is more about moving data over a physical link, guaranteeing
>> >> >> ordered delivery and avoiding unnecessary transmissions if remote
>> >> >> doesn't have the resources, it's much lower level than Greybus.
>> >> >
>> >> > As is said, i don't know Greybus too well. I hope its Maintainers can
>> >> > comment on this.
>> >> >
>> >> >> > Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
>> >> >> > the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
>> >> >> > has support for these, although the code is current in staging. But
>> >> >> > for staging code, it is actually pretty good.
>> >> >>
>> >> >> I agree with you that the EFR32 is a general purpose SoC and exposing
>> >> >> all available peripherals would be great, but most customers buy it as
>> >> >> an RCP module with one or more radio stacks enabled, and that's the
>> >> >> situation we're trying to address. Maybe I introduced a framework with
>> >> >> custom bus, drivers and endpoints where it was unnecessary, the goal is
>> >> >> not to be super generic but only to support coexistence of our radio
>> >> >> stacks.
>> >> >
>> >> > This leads to my next problem.
>> >> >
>> >> > https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
>> >> > Nordic Semiconductor has what appears to be a similar device.
>> >> >
>> >> > https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
>> >> > Microchip has a similar device as well.
>> >> >
>> >> > https://www.ti.com/product/CC2674R10
>> >> > TI has a similar device.
>> >> >
>> >> > And maybe there are others?
>> >> >
>> >> > Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
>> >> > CPC, and an ACME CPC?
>> >> >
>> >> > How do we end up with one implementation?
>> >> >
>> >> > Maybe Greybus does not currently support your streaming use case too
>> >> > well, but it is at least vendor neutral. Can it be extended for
>> >> > streaming?
>> >>
>> >> I get the sentiment that we don't want every single vendor to push their
>> >> own protocols that are ever so slightly different. To be honest, I don't
>> >> know if Greybus can be extended for that use case, or if it's something
>> >> they are interested in supporting. I've subscribed to greybus-dev so
>> >> hopefully my email will get through this time (previous one is pending
>> >> approval).
>> >>
>> >> Unfortunately, we're deep down the CPC road, especially on the firmware
>> >> side. Blame on me for not sending the RFC sooner and getting feedback
>> >> earlier, but if we have to massively change our course of action we need
>> >> some degree of confidence that this is a viable alternative for
>> >> achieving high-throughput for WiFi over SDIO. I would really value any
>> >> input from the Greybus folks on this.
>> >
>> > So what you are looking for is a standard way to "tunnel" SDIO over some
>> > other physical transport, right? If so, then yes, please use Greybus as
>> > that is exactly what it was designed for.
>>
>> No, we want to use SDIO as physical transport. To use the Greybus
>> terminology, our MCUs would act as modules with a single interface, and
>> that interface would have "radio" bundles for each of the supported
>> stack.
>>
>> So we want to expose our radio stacks in Linux and Greybus doesn't
>> define protocols for that, so that's kind of uncharted territories and
>> we were wondering if Greybus would be the right tool for that. I hope
>> the situation is a bit clearer now.
>
> Yes, greybus does not expose a "wifi" protocol as that is way too device
> specific, sorry.
>
> So this just would be like any other normal SDIO wifi device then,
> shouldn't be anything special, right?
Wifi is just one of the radio stacks that can be present but there can
be other radio stacks running on the same device and sharing the same
physical transport, like Bluetooth, Zigbee, or OpenThread. The goal of
CPC (our custom protocol) is to multiplex all these protocols over the
same physical bus.
I think Andrew pulled Greybus in the discussion because there is some
overlap between Greybus and CPC:
- Greybus has bundles and CPorts, CPC only has "endpoints", which
would be the equivalent of a bundle with a single cport
- discoverability of Greybus bundles/CPC endpoints by the host
- multiple bundles/endpoints might coexist in the same
module/CPC-enabled device
- bundles/endpoints are independent from each other and each has its
own dedicated driver
Greybus goes a step further and specs some protocols like GPIO or UART.
CPC doesn't spec what goes over endpoints because it's geared towards
radio applications and as you said, it's very device/stack specific.
Once an endpoint is connected, CPC just passes a bidirectionnal stream
of data between the two ends, which are free to do whatever they want
with it. A good example of that is the bluetooth driver that's part of
this RFC [1]. I hope my explanations make sense.
[1] https://lore.kernel.org/netdev/20250512012748.79749-16-damien.riegel@silabs.com/T/#u
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-16 16:25 ` Damien Riégel
@ 2025-05-18 15:23 ` Andrew Lunn
2025-05-20 1:21 ` Damien Riégel
2025-05-22 2:46 ` Alex Elder
0 siblings, 2 replies; 39+ messages in thread
From: Andrew Lunn @ 2025-05-18 15:23 UTC (permalink / raw)
To: Damien Riégel
Cc: Greg Kroah-Hartman, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
> I think Andrew pulled Greybus in the discussion because there is some
> overlap between Greybus and CPC:
> - Greybus has bundles and CPorts, CPC only has "endpoints", which
> would be the equivalent of a bundle with a single cport
> - discoverability of Greybus bundles/CPC endpoints by the host
> - multiple bundles/endpoints might coexist in the same
> module/CPC-enabled device
> - bundles/endpoints are independent from each other and each has its
> own dedicated driver
>
> Greybus goes a step further and specs some protocols like GPIO or UART.
> CPC doesn't spec what goes over endpoints because it's geared towards
> radio applications and as you said, it's very device/stack specific.
Is it device specific? Look at your Bluetooth implementation. I don't
see anything device specific in it. That should work for any of the
vendors of similar chips to yours.
For 802.15.4, Linux defines:
struct ieee802154_ops {
struct module *owner;
int (*start)(struct ieee802154_hw *hw);
void (*stop)(struct ieee802154_hw *hw);
int (*xmit_sync)(struct ieee802154_hw *hw,
struct sk_buff *skb);
int (*xmit_async)(struct ieee802154_hw *hw,
struct sk_buff *skb);
int (*ed)(struct ieee802154_hw *hw, u8 *level);
int (*set_channel)(struct ieee802154_hw *hw, u8 page,
u8 channel);
int (*set_hw_addr_filt)(struct ieee802154_hw *hw,
struct ieee802154_hw_addr_filt *filt,
unsigned long changed);
int (*set_txpower)(struct ieee802154_hw *hw, s32 mbm);
int (*set_lbt)(struct ieee802154_hw *hw, bool on);
int (*set_cca_mode)(struct ieee802154_hw *hw,
const struct wpan_phy_cca *cca);
int (*set_cca_ed_level)(struct ieee802154_hw *hw, s32 mbm);
int (*set_csma_params)(struct ieee802154_hw *hw,
u8 min_be, u8 max_be, u8 retries);
int (*set_frame_retries)(struct ieee802154_hw *hw,
s8 retries);
int (*set_promiscuous_mode)(struct ieee802154_hw *hw,
const bool on);
};
Many of these are optional, but this gives an abstract representation
of a device, which is should be possible to turn into a protocol
talked over a transport bus like SPI or SDIO.
This also comes back to my point of there being at least four vendors
of devices like yours. Linux does not want four or more
implementations of this, each 90% the same, just a different way of
converting this structure of operations into messages over a transport
bus.
You have to define the protocol. Mainline needs that so when the next
vendor comes along, we can point at your protocol and say that is how
it has to be implemented in Mainline. Make your firmware on the SoC
understand it. You have the advantage that you are here first, you
get to define that protocol, but you do need to clearly define it.
You have listed how your implementation is similar to Greybus. You say
what is not so great is streaming, i.e. the bulk data transfer needed
to implement xmit_sync() and xmit_async() above. Greybus is too much
RPC based. RPCs are actually what you want for most of the operations
listed above, but i agree for data, in order to keep the transport
fully loaded, you want double buffering. However, that appears to be
possible with the current Greybus code.
gb_operation_unidirectional_timeout() says:
* Note that successful send of a unidirectional operation does not imply that
* the request as actually reached the remote end of the connection.
*/
So long as you are doing your memory management correctly, i don't see
why you cannot implement double buffering in the transport driver.
I also don't see why you cannot extend the Greybus upper API and add a
true gb_operation_unidirectional_async() call.
You also said that lots of small transfers are inefficient, and you
wanted to combine small high level messages into one big transport
layer message. This is something you frequently see with USB Ethernet
dongles. The Ethernet driver puts a number of small Ethernet packets
into one USB URB. The USB layer itself has no idea this is going on. I
don't see why the same cannot be done here, greybus itself does not
need to be aware of the packet consolidation.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-18 15:23 ` Andrew Lunn
@ 2025-05-20 1:21 ` Damien Riégel
2025-05-20 13:04 ` Andrew Lunn
2025-05-22 2:46 ` Alex Elder
1 sibling, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-20 1:21 UTC (permalink / raw)
To: Andrew Lunn
Cc: Greg Kroah-Hartman, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On Sun May 18, 2025 at 11:23 AM EDT, Andrew Lunn wrote:
> This also comes back to my point of there being at least four vendors
> of devices like yours. Linux does not want four or more
> implementations of this, each 90% the same, just a different way of
> converting this structure of operations into messages over a transport
> bus.
>
> You have to define the protocol. Mainline needs that so when the next
> vendor comes along, we can point at your protocol and say that is how
> it has to be implemented in Mainline. Make your firmware on the SoC
> understand it. You have the advantage that you are here first, you
> get to define that protocol, but you do need to clearly define it.
I understand that this is the preferred way and I'll push internally for
going that direction. That being said, Greybus seems to offer the
capability to have a custom driver for a given PID/VID, if a module
doesn't implement a Greybus-standardized protocol. Would a custom
Greybus driver for, just as an example, our Wifi stack be an acceptable
option?
> You have listed how your implementation is similar to Greybus. You say
> what is not so great is streaming, i.e. the bulk data transfer needed
> to implement xmit_sync() and xmit_async() above. Greybus is too much
> RPC based. RPCs are actually what you want for most of the operations
> listed above, but i agree for data, in order to keep the transport
> fully loaded, you want double buffering. However, that appears to be
> possible with the current Greybus code.
>
> gb_operation_unidirectional_timeout() says:
>
> * Note that successful send of a unidirectional operation does not imply that
> * the request as actually reached the remote end of the connection.
> */
>
> So long as you are doing your memory management correctly, i don't see
> why you cannot implement double buffering in the transport driver.
>
> I also don't see why you cannot extend the Greybus upper API and add a
> true gb_operation_unidirectional_async() call.
Just because touching a well established subsystem is scary, but I
understand that we're allowed to make changes that make sense.
> You also said that lots of small transfers are inefficient, and you
> wanted to combine small high level messages into one big transport
> layer message. This is something you frequently see with USB Ethernet
> dongles. The Ethernet driver puts a number of small Ethernet packets
> into one USB URB. The USB layer itself has no idea this is going on. I
> don't see why the same cannot be done here, greybus itself does not
> need to be aware of the packet consolidation.
Yeah, so in this design, CPC would really be limited to the transport
bus (SPI for now), to do packet consolidation and managing RCP available
buffers. I think at this point, the next step is to come up with a proof
of concept of Greybus over CPC and see if that works or not.
Let me add that I sincerely appreciate that you took the time to review
this RFC and provided an upstream-compatible alternative to what we
proposed, so thank you for that.
Damien
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-20 1:21 ` Damien Riégel
@ 2025-05-20 13:04 ` Andrew Lunn
2025-05-22 2:46 ` Alex Elder
0 siblings, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-05-20 13:04 UTC (permalink / raw)
To: Damien Riégel
Cc: Greg Kroah-Hartman, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On Mon, May 19, 2025 at 09:21:52PM -0400, Damien Riégel wrote:
> On Sun May 18, 2025 at 11:23 AM EDT, Andrew Lunn wrote:
> > This also comes back to my point of there being at least four vendors
> > of devices like yours. Linux does not want four or more
> > implementations of this, each 90% the same, just a different way of
> > converting this structure of operations into messages over a transport
> > bus.
> >
> > You have to define the protocol. Mainline needs that so when the next
> > vendor comes along, we can point at your protocol and say that is how
> > it has to be implemented in Mainline. Make your firmware on the SoC
> > understand it. You have the advantage that you are here first, you
> > get to define that protocol, but you do need to clearly define it.
>
> I understand that this is the preferred way and I'll push internally for
> going that direction. That being said, Greybus seems to offer the
> capability to have a custom driver for a given PID/VID, if a module
> doesn't implement a Greybus-standardized protocol. Would a custom
> Greybus driver for, just as an example, our Wifi stack be an acceptable
> option?
It is not clear to me why a custom driver would be needed. You need to
implement a Linux WiFi driver. That API is well defined, although you
might only need a subset. What do you need in addition to that?
> > So long as you are doing your memory management correctly, i don't see
> > why you cannot implement double buffering in the transport driver.
> >
> > I also don't see why you cannot extend the Greybus upper API and add a
> > true gb_operation_unidirectional_async() call.
>
> Just because touching a well established subsystem is scary, but I
> understand that we're allowed to make changes that make sense.
There are developers here to help review such changes. And extending
existing Linux subsystems is how Linux has become the dominant OS. You
are getting it for free, building on the work of others, so it is not
too unreasonable to contribute a little bit back by making it even
better.
>
> > You also said that lots of small transfers are inefficient, and you
> > wanted to combine small high level messages into one big transport
> > layer message. This is something you frequently see with USB Ethernet
> > dongles. The Ethernet driver puts a number of small Ethernet packets
> > into one USB URB. The USB layer itself has no idea this is going on. I
> > don't see why the same cannot be done here, greybus itself does not
> > need to be aware of the packet consolidation.
>
> Yeah, so in this design, CPC would really be limited to the transport
> bus (SPI for now), to do packet consolidation and managing RCP available
> buffers. I think at this point, the next step is to come up with a proof
> of concept of Greybus over CPC and see if that works or not.
You need to keep the lower level generic. I would not expect anything
Silabs specific in how you transport Greybus over SPI or SDIO. As part
of gb_operation_unidirectional_async() you need to think about flow
control, you need some generic mechanism to indicate receive buffer
availability in the device, and when to pause a while to let the
device catch up, but there is no reason TI, Microchip, Nordic, etc
should not be able to use the same encapsulation scheme.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-20 13:04 ` Andrew Lunn
@ 2025-05-22 2:46 ` Alex Elder
0 siblings, 0 replies; 39+ messages in thread
From: Alex Elder @ 2025-05-22 2:46 UTC (permalink / raw)
To: Andrew Lunn, Damien Riégel
Cc: Greg Kroah-Hartman, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On 5/20/25 8:04 AM, Andrew Lunn wrote:
> On Mon, May 19, 2025 at 09:21:52PM -0400, Damien Riégel wrote:
>> On Sun May 18, 2025 at 11:23 AM EDT, Andrew Lunn wrote:
>>> This also comes back to my point of there being at least four vendors
>>> of devices like yours. Linux does not want four or more
>>> implementations of this, each 90% the same, just a different way of
>>> converting this structure of operations into messages over a transport
>>> bus.
>>>
>>> You have to define the protocol. Mainline needs that so when the next
>>> vendor comes along, we can point at your protocol and say that is how
>>> it has to be implemented in Mainline. Make your firmware on the SoC
>>> understand it. You have the advantage that you are here first, you
>>> get to define that protocol, but you do need to clearly define it.
>>
>> I understand that this is the preferred way and I'll push internally for
>> going that direction. That being said, Greybus seems to offer the
>> capability to have a custom driver for a given PID/VID, if a module
>> doesn't implement a Greybus-standardized protocol. Would a custom
>> Greybus driver for, just as an example, our Wifi stack be an acceptable
>> option?
>
> It is not clear to me why a custom driver would be needed. You need to
> implement a Linux WiFi driver. That API is well defined, although you
> might only need a subset. What do you need in addition to that?
This "custom driver" is needed for CPC too, right?
You need some way to translate what's happening in
the kernel into directions sent over your transport
to the hardware on the other side.
Don't worry about proposing changes to Greybus. But
please do it incrementally, and share what you would
like to do, so people can help steer you in the most
promising direction.
-Alex
>>> So long as you are doing your memory management correctly, i don't see
>>> why you cannot implement double buffering in the transport driver.
>>>
>>> I also don't see why you cannot extend the Greybus upper API and add a
>>> true gb_operation_unidirectional_async() call.
>>
>> Just because touching a well established subsystem is scary, but I
>> understand that we're allowed to make changes that make sense.
>
> There are developers here to help review such changes. And extending
> existing Linux subsystems is how Linux has become the dominant OS. You
> are getting it for free, building on the work of others, so it is not
> too unreasonable to contribute a little bit back by making it even
> better.
>
>>
>>> You also said that lots of small transfers are inefficient, and you
>>> wanted to combine small high level messages into one big transport
>>> layer message. This is something you frequently see with USB Ethernet
>>> dongles. The Ethernet driver puts a number of small Ethernet packets
>>> into one USB URB. The USB layer itself has no idea this is going on. I
>>> don't see why the same cannot be done here, greybus itself does not
>>> need to be aware of the packet consolidation.
>>
>> Yeah, so in this design, CPC would really be limited to the transport
>> bus (SPI for now), to do packet consolidation and managing RCP available
>> buffers. I think at this point, the next step is to come up with a proof
>> of concept of Greybus over CPC and see if that works or not.
>
> You need to keep the lower level generic. I would not expect anything
> Silabs specific in how you transport Greybus over SPI or SDIO. As part
> of gb_operation_unidirectional_async() you need to think about flow
> control, you need some generic mechanism to indicate receive buffer
> availability in the device, and when to pause a while to let the
> device catch up, but there is no reason TI, Microchip, Nordic, etc
> should not be able to use the same encapsulation scheme.
>
> Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-18 15:23 ` Andrew Lunn
2025-05-20 1:21 ` Damien Riégel
@ 2025-05-22 2:46 ` Alex Elder
2025-05-22 18:11 ` Andrew Lunn
1 sibling, 1 reply; 39+ messages in thread
From: Alex Elder @ 2025-05-22 2:46 UTC (permalink / raw)
To: Andrew Lunn, Damien Riégel
Cc: Greg Kroah-Hartman, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, greybus-dev
On 5/18/25 10:23 AM, Andrew Lunn wrote:
>> I think Andrew pulled Greybus in the discussion because there is some
>> overlap between Greybus and CPC:
>> - Greybus has bundles and CPorts, CPC only has "endpoints", which
>> would be the equivalent of a bundle with a single cport
>> - discoverability of Greybus bundles/CPC endpoints by the host
>> - multiple bundles/endpoints might coexist in the same
>> module/CPC-enabled device
>> - bundles/endpoints are independent from each other and each has its
>> own dedicated driver
>>
>> Greybus goes a step further and specs some protocols like GPIO or UART.
>> CPC doesn't spec what goes over endpoints because it's geared towards
>> radio applications and as you said, it's very device/stack specific.
>
> Is it device specific? Look at your Bluetooth implementation. I don't
> see anything device specific in it. That should work for any of the
> vendors of similar chips to yours.
>
> For 802.15.4, Linux defines:
>
> struct ieee802154_ops {
> struct module *owner;
> int (*start)(struct ieee802154_hw *hw);
> void (*stop)(struct ieee802154_hw *hw);
> int (*xmit_sync)(struct ieee802154_hw *hw,
> struct sk_buff *skb);
> int (*xmit_async)(struct ieee802154_hw *hw,
> struct sk_buff *skb);
> int (*ed)(struct ieee802154_hw *hw, u8 *level);
> int (*set_channel)(struct ieee802154_hw *hw, u8 page,
> u8 channel);
> int (*set_hw_addr_filt)(struct ieee802154_hw *hw,
> struct ieee802154_hw_addr_filt *filt,
> unsigned long changed);
> int (*set_txpower)(struct ieee802154_hw *hw, s32 mbm);
> int (*set_lbt)(struct ieee802154_hw *hw, bool on);
> int (*set_cca_mode)(struct ieee802154_hw *hw,
> const struct wpan_phy_cca *cca);
> int (*set_cca_ed_level)(struct ieee802154_hw *hw, s32 mbm);
> int (*set_csma_params)(struct ieee802154_hw *hw,
> u8 min_be, u8 max_be, u8 retries);
> int (*set_frame_retries)(struct ieee802154_hw *hw,
> s8 retries);
> int (*set_promiscuous_mode)(struct ieee802154_hw *hw,
> const bool on);
> };
>
> Many of these are optional, but this gives an abstract representation
> of a device, which is should be possible to turn into a protocol
> talked over a transport bus like SPI or SDIO.
This is essentially how Greybus does things. It sets
up drivers on the Linux side that translate callback
functions into Greybus operations that get performed
on target hardware on the remote module.
> This also comes back to my point of there being at least four vendors
> of devices like yours. Linux does not want four or more
> implementations of this, each 90% the same, just a different way of
> converting this structure of operations into messages over a transport
> bus.
>
> You have to define the protocol. Mainline needs that so when the next
> vendor comes along, we can point at your protocol and say that is how
> it has to be implemented in Mainline. Make your firmware on the SoC
> understand it. You have the advantage that you are here first, you
> get to define that protocol, but you do need to clearly define it.
I agree with all of this.
> You have listed how your implementation is similar to Greybus. You say
> what is not so great is streaming, i.e. the bulk data transfer needed
> to implement xmit_sync() and xmit_async() above. Greybus is too much
> RPC based. RPCs are actually what you want for most of the operations
> listed above, but i agree for data, in order to keep the transport
> fully loaded, you want double buffering. However, that appears to be
> possible with the current Greybus code.
>
> gb_operation_unidirectional_timeout() says:
Yes, these are request messages that don't require a response.
The acknowledgement is about when the host *sent it*, not when
it got received. They're rarely used but I could see them being
used this way. Still, you might be limited to 255 or so in-flight
messages.
-Alex
> * Note that successful send of a unidirectional operation does not imply that
> * the request as actually reached the remote end of the connection.
> */
>
> So long as you are doing your memory management correctly, i don't see
> why you cannot implement double buffering in the transport driver.
>
> I also don't see why you cannot extend the Greybus upper API and add a
> true gb_operation_unidirectional_async() call.
>
> You also said that lots of small transfers are inefficient, and you
> wanted to combine small high level messages into one big transport
> layer message. This is something you frequently see with USB Ethernet
> dongles. The Ethernet driver puts a number of small Ethernet packets
> into one USB URB. The USB layer itself has no idea this is going on. I
> don't see why the same cannot be done here, greybus itself does not
> need to be aware of the packet consolidation.
>
> Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-22 2:46 ` Alex Elder
@ 2025-05-22 18:11 ` Andrew Lunn
0 siblings, 0 replies; 39+ messages in thread
From: Andrew Lunn @ 2025-05-22 18:11 UTC (permalink / raw)
To: Alex Elder
Cc: Damien Riégel, Greg Kroah-Hartman, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel,
Johan Hovold, Alex Elder, greybus-dev
> > You have listed how your implementation is similar to Greybus. You say
> > what is not so great is streaming, i.e. the bulk data transfer needed
> > to implement xmit_sync() and xmit_async() above. Greybus is too much
> > RPC based. RPCs are actually what you want for most of the operations
> > listed above, but i agree for data, in order to keep the transport
> > fully loaded, you want double buffering. However, that appears to be
> > possible with the current Greybus code.
> >
> > gb_operation_unidirectional_timeout() says:
>
> Yes, these are request messages that don't require a response.
> The acknowledgement is about when the host *sent it*, not when
> it got received. They're rarely used but I could see them being
> used this way. Still, you might be limited to 255 or so in-flight
> messages.
I don't actually see how you can have multiple messages in-flight, but
maybe i'm missing something. It appears that upper layers pass the
message down and then block on a completion. The signalling of that
completion only happens when the message is on the wire. So it is all
synchronous. In order to have multiple messages in-flight, the lower
layer would have to copy the message, signal the completion, and then
send the copy whenever the transport was free.
The network stack is however async by nature. The ndo_start_xmit call
passes an skbuf. The data in the skbuf is setup for DMA transfer, and
then ndo_start_xmit returns. Later, when the DMA has completed, the
driver calls dev_kfree_skb() to say it has finished with the skb.
Ideally we want a similar async mechanism, which is why i suggested
gb_operation_unidirectional_async(). Pass a message to Greybus, none
blocking, and have a callback for when the message has hit the wire
and the skb can be friend. The low level can then keep a list of skb's
so it can quickly do back to back transfers over the transport to keep
it busy.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-14 22:52 ` Damien Riégel
2025-05-15 7:49 ` Greg Kroah-Hartman
@ 2025-05-22 2:46 ` Alex Elder
2025-05-23 19:49 ` Damien Riégel
1 sibling, 1 reply; 39+ messages in thread
From: Alex Elder @ 2025-05-22 2:46 UTC (permalink / raw)
To: Damien Riégel, Andrew Lunn
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, greybus-dev
On 5/14/25 5:52 PM, Damien Riégel wrote:
> On Tue May 13, 2025 at 5:53 PM EDT, Andrew Lunn wrote:
>> On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
>>> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
>>>> On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
>>>>> Hi,
>>>>>
>>>>>
>>>>> This patchset brings initial support for Silicon Labs CPC protocol,
>>>>> standing for Co-Processor Communication. This protocol is used by the
>>>>> EFR32 Series [1]. These devices offer a variety for radio protocols,
>>>>> such as Bluetooth, Z-Wave, Zigbee [2].
>>>>
>>>> Before we get too deep into the details of the patches, please could
>>>> you do a compare/contrast to Greybus.
>>>
>>> Thank you for the prompt feedback on the RFC. We took a look at Greybus
>>> in the past and it didn't seem to fit our needs. One of the main use
>>> case that drove the development of CPC was to support WiFi (in
>>> coexistence with other radio stacks) over SDIO, and get the maximum
>>> throughput possible. We concluded that to achieve this we would need
>>> packet aggregation, as sending one frame at a time over SDIO is
>>> wasteful, and managing Radio Co-Processor available buffers, as sending
>>> frames that the RCP is not able to process would degrade performance.
>>>
>>> Greybus don't seem to offer these capabilities. It seems to be more
>>> geared towards implementing RPC, where the host would send a command,
>>> and then wait for the device to execute it and to respond. For Greybus'
>>> protocols that implement some "streaming" features like audio or video
>>> capture, the data streams go to an I2S or CSI interface, but it doesn't
>>> seem to go through a CPort. So it seems to act as a backbone to connect
>>> CPorts together, but high-throughput transfers happen on other types of
>>> links. CPC is more about moving data over a physical link, guaranteeing
>>> ordered delivery and avoiding unnecessary transmissions if remote
>>> doesn't have the resources, it's much lower level than Greybus.
>>
>> As is said, i don't know Greybus too well. I hope its Maintainers can
>> comment on this.
>>
>>>> Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
>>>> the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
>>>> has support for these, although the code is current in staging. But
>>>> for staging code, it is actually pretty good.
>>>
>>> I agree with you that the EFR32 is a general purpose SoC and exposing
>>> all available peripherals would be great, but most customers buy it as
>>> an RCP module with one or more radio stacks enabled, and that's the
>>> situation we're trying to address. Maybe I introduced a framework with
>>> custom bus, drivers and endpoints where it was unnecessary, the goal is
>>> not to be super generic but only to support coexistence of our radio
>>> stacks.
>>
>> This leads to my next problem.
>>
>> https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
>> Nordic Semiconductor has what appears to be a similar device.
>>
>> https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
>> Microchip has a similar device as well.
>>
>> https://www.ti.com/product/CC2674R10
>> TI has a similar device.
>>
>> And maybe there are others?
>>
>> Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
>> CPC, and an ACME CPC?
>>
>> How do we end up with one implementation?
>>
>> Maybe Greybus does not currently support your streaming use case too
>> well, but it is at least vendor neutral. Can it be extended for
>> streaming?
>
> I get the sentiment that we don't want every single vendor to push their
> own protocols that are ever so slightly different. To be honest, I don't
> know if Greybus can be extended for that use case, or if it's something
> they are interested in supporting. I've subscribed to greybus-dev so
> hopefully my email will get through this time (previous one is pending
> approval).
Greybus was designed for a particular platform, but the intention
was to make it extensible. It can be extended with new protocols,
and I don't think anyone is opposed to that.
> Unfortunately, we're deep down the CPC road, especially on the firmware
> side. Blame on me for not sending the RFC sooner and getting feedback
> earlier, but if we have to massively change our course of action we need
> some degree of confidence that this is a viable alternative for
> achieving high-throughput for WiFi over SDIO. I would really value any
> input from the Greybus folks on this.
I kind of assumed this. I'm sure Andrew's message was not that
welcome for that reason, but he's right about trying to agree on
something in common if possible. If Greybus can solve all your
problems, the maintainers will support the code being modified
to support what's needed.
(To be clear, I don't assume Greybus will solve all your problems.
For example, UniPro provides a reliable transport, so that's what
Greybus currently expects.)
I have no input on your throughput question at the moment.
-Alex
>> And maybe a dumb question... How do transfers get out of order over
>> SPI and SDIO? If you look at the Open Alliance TC6 specification for
>> Ethernet over SPI, it does not have any issues with ordering.
>
> Sorry I wasn't very clear about that. Of course packets are sent in
> order but several packets can be sent at once before being acknowledged
> and we might detect CRC errors on one of these packets. CPC takes care
> of only delivering valid packets, and packets that come after the one
> with CRC error won't be delivered to upper layer until the faulty one is
> retransmitted.
>
> I took a look at the specification you mentioned and they completely
> delegate that to upper layers:
>
> When transmit or receive frame bit errors are detected on the SPI,
> the retry of frames is performed by higher protocol layers that are
> beyond the scope of this specification. [1]
>
> Our goal was to be agnostic of stacks on top of CPC and reliably
> transmit frames. To give a bit of context, CPC was originally derived
> from HDLC, which features detecting sequence gaps and retransmission. On
> top of that, we've now added the mechanism I mentioned in previous
> emails that throttle the host when the RCP is not ready to receive and
> process frames on an endpoint.
>
> [1] https://opensig.org/wp-content/uploads/2023/12/OPEN_Alliance_10BASET1x_MAC-PHY_Serial_Interface_V1.1.pdf (Section 7.3.1)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-22 2:46 ` Alex Elder
@ 2025-05-23 19:49 ` Damien Riégel
2025-05-23 20:06 ` Andrew Lunn
0 siblings, 1 reply; 39+ messages in thread
From: Damien Riégel @ 2025-05-23 19:49 UTC (permalink / raw)
To: Alex Elder, Andrew Lunn
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Silicon Labs Kernel Team, netdev, devicetree, linux-kernel,
Johan Hovold, Alex Elder, Greg Kroah-Hartman, greybus-dev
On Wed May 21, 2025 at 10:46 PM EDT, Alex Elder wrote:
> On 5/14/25 5:52 PM, Damien Riégel wrote:
>> On Tue May 13, 2025 at 5:53 PM EDT, Andrew Lunn wrote:
>>> On Tue, May 13, 2025 at 05:15:20PM -0400, Damien Riégel wrote:
>>>> On Mon May 12, 2025 at 1:07 PM EDT, Andrew Lunn wrote:
>>>>> On Sun, May 11, 2025 at 09:27:33PM -0400, Damien Riégel wrote:
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> This patchset brings initial support for Silicon Labs CPC protocol,
>>>>>> standing for Co-Processor Communication. This protocol is used by the
>>>>>> EFR32 Series [1]. These devices offer a variety for radio protocols,
>>>>>> such as Bluetooth, Z-Wave, Zigbee [2].
>>>>>
>>>>> Before we get too deep into the details of the patches, please could
>>>>> you do a compare/contrast to Greybus.
>>>>
>>>> Thank you for the prompt feedback on the RFC. We took a look at Greybus
>>>> in the past and it didn't seem to fit our needs. One of the main use
>>>> case that drove the development of CPC was to support WiFi (in
>>>> coexistence with other radio stacks) over SDIO, and get the maximum
>>>> throughput possible. We concluded that to achieve this we would need
>>>> packet aggregation, as sending one frame at a time over SDIO is
>>>> wasteful, and managing Radio Co-Processor available buffers, as sending
>>>> frames that the RCP is not able to process would degrade performance.
>>>>
>>>> Greybus don't seem to offer these capabilities. It seems to be more
>>>> geared towards implementing RPC, where the host would send a command,
>>>> and then wait for the device to execute it and to respond. For Greybus'
>>>> protocols that implement some "streaming" features like audio or video
>>>> capture, the data streams go to an I2S or CSI interface, but it doesn't
>>>> seem to go through a CPort. So it seems to act as a backbone to connect
>>>> CPorts together, but high-throughput transfers happen on other types of
>>>> links. CPC is more about moving data over a physical link, guaranteeing
>>>> ordered delivery and avoiding unnecessary transmissions if remote
>>>> doesn't have the resources, it's much lower level than Greybus.
>>>
>>> As is said, i don't know Greybus too well. I hope its Maintainers can
>>> comment on this.
>>>
>>>>> Also, this patch adds Bluetooth, you talk about Z-Wave and Zigbee. But
>>>>> the EFR32 is a general purpose SoC, with I2C, SPI, PWM, UART. Greybus
>>>>> has support for these, although the code is current in staging. But
>>>>> for staging code, it is actually pretty good.
>>>>
>>>> I agree with you that the EFR32 is a general purpose SoC and exposing
>>>> all available peripherals would be great, but most customers buy it as
>>>> an RCP module with one or more radio stacks enabled, and that's the
>>>> situation we're trying to address. Maybe I introduced a framework with
>>>> custom bus, drivers and endpoints where it was unnecessary, the goal is
>>>> not to be super generic but only to support coexistence of our radio
>>>> stacks.
>>>
>>> This leads to my next problem.
>>>
>>> https://www.nordicsemi.com/-/media/Software-and-other-downloads/Product-Briefs/nRF5340-SoC-PB.pdf
>>> Nordic Semiconductor has what appears to be a similar device.
>>>
>>> https://www.microchip.com/en-us/products/wireless-connectivity/bluetooth-low-energy/microcontrollers
>>> Microchip has a similar device as well.
>>>
>>> https://www.ti.com/product/CC2674R10
>>> TI has a similar device.
>>>
>>> And maybe there are others?
>>>
>>> Are we going to get a Silabs CPC, a Nordic CPC, a Microchip CPC, a TI
>>> CPC, and an ACME CPC?
>>>
>>> How do we end up with one implementation?
>>>
>>> Maybe Greybus does not currently support your streaming use case too
>>> well, but it is at least vendor neutral. Can it be extended for
>>> streaming?
>>
>> I get the sentiment that we don't want every single vendor to push their
>> own protocols that are ever so slightly different. To be honest, I don't
>> know if Greybus can be extended for that use case, or if it's something
>> they are interested in supporting. I've subscribed to greybus-dev so
>> hopefully my email will get through this time (previous one is pending
>> approval).
>
> Greybus was designed for a particular platform, but the intention
> was to make it extensible. It can be extended with new protocols,
> and I don't think anyone is opposed to that.
>
>> Unfortunately, we're deep down the CPC road, especially on the firmware
>> side. Blame on me for not sending the RFC sooner and getting feedback
>> earlier, but if we have to massively change our course of action we need
>> some degree of confidence that this is a viable alternative for
>> achieving high-throughput for WiFi over SDIO. I would really value any
>> input from the Greybus folks on this.
>
> I kind of assumed this. I'm sure Andrew's message was not that
> welcome for that reason, but he's right about trying to agree on
> something in common if possible. If Greybus can solve all your
> problems, the maintainers will support the code being modified
> to support what's needed.
>
> (To be clear, I don't assume Greybus will solve all your problems.
> For example, UniPro provides a reliable transport, so that's what
> Greybus currently expects.)
I don't really know about UniPro and I'm learning about it as the
discussion goes, but one of the point listed on Wikipedia is
"reliability - data errors detected and correctable via retransmission"
This is where CPC could come in, probably with a different name and a
reduced scope, as a way to implement reliable transmission over UART,
SPI, SDIO, by ensuring data errors are detected and packets
retransmitted if necessary, and be limited to that.
What's missing for us in Greybus, as discussed in a subthread, is
asynchronous operations to fit better with the network stack, but I
think that could easily be added.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-23 19:49 ` Damien Riégel
@ 2025-05-23 20:06 ` Andrew Lunn
2025-05-23 20:38 ` Damien Riégel
0 siblings, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-05-23 20:06 UTC (permalink / raw)
To: Damien Riégel
Cc: Alex Elder, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, Greg Kroah-Hartman,
greybus-dev
> I don't really know about UniPro and I'm learning about it as the
> discussion goes, but one of the point listed on Wikipedia is
> "reliability - data errors detected and correctable via retransmission"
>
> This is where CPC could come in, probably with a different name and a
> reduced scope, as a way to implement reliable transmission over UART,
> SPI, SDIO, by ensuring data errors are detected and packets
> retransmitted if necessary, and be limited to that.
You mentioned HDLC in the past. What is interesting is that HDLC is
actually used in Greybus:
https://elixir.bootlin.com/linux/v6.15-rc7/source/drivers/greybus/gb-beagleplay.c#L581
I've no idea if its just for framing, or if there is also retries on
errors, S-frames with flow and error control etc. There might be code
you can reuse here.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC net-next 00/15] Add support for Silicon Labs CPC
2025-05-23 20:06 ` Andrew Lunn
@ 2025-05-23 20:38 ` Damien Riégel
0 siblings, 0 replies; 39+ messages in thread
From: Damien Riégel @ 2025-05-23 20:38 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alex Elder, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Silicon Labs Kernel Team, netdev, devicetree,
linux-kernel, Johan Hovold, Alex Elder, Greg Kroah-Hartman,
greybus-dev
On Fri May 23, 2025 at 4:06 PM EDT, Andrew Lunn wrote:
>> I don't really know about UniPro and I'm learning about it as the
>> discussion goes, but one of the point listed on Wikipedia is
>> "reliability - data errors detected and correctable via retransmission"
>>
>> This is where CPC could come in, probably with a different name and a
>> reduced scope, as a way to implement reliable transmission over UART,
>> SPI, SDIO, by ensuring data errors are detected and packets
>> retransmitted if necessary, and be limited to that.
>
> You mentioned HDLC in the past. What is interesting is that HDLC is
> actually used in Greybus:
>
> https://elixir.bootlin.com/linux/v6.15-rc7/source/drivers/greybus/gb-beagleplay.c*L581
>
> I've no idea if its just for framing, or if there is also retries on
> errors, S-frames with flow and error control etc. There might be code
> you can reuse here.
Yeah I've seen it when looking at Greybus, from what I could see it's
only framing. There is a CRC check though, frames received that don't
pass that check are not passed to Greybus layer.
Another aspect we would like to support is buffer management. In our
implementation, each endpoint has its dedicated pool of RX buffers and
the number of available buffers is advertised to the remote, so the
Linux driver can delay transmission of packets if endpoints are out of
RX buffers.
We decide to implement that mostly because that would get us the best
throughtput possible. Sending a packet to an endpoint that doesn't have
room for it means the packet will be dropped and we have to wait for a
retransmission to occur, which degrades performance.
^ permalink raw reply [flat|nested] 39+ messages in thread