Modem CMUX: Optimize away work buf

doc/releases/migration-guide-4.3.rst

@@ -55,6 +55,20 @@ Other subsystems
 
 .. zephyr-keep-sorted-start re(^\w)
 
+Modem Modules
+=============
+
+* Removed :kconfig:option:`CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE`. The option was being misused to
+  determine unrelated buffer sizes and the cmux work buffer itself is no longer needed.
+
+  If the option :kconfig:option:`CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE` was used to determine the
+  size of the ``receive_buf`` member of a :c:struct:`modem_cmux_config` instance, replace it
+  with ``CONFIG_MODEM_CMUX_MTU + 7``.
+
+  If the option :kconfig:option:`CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE` was used to determine the
+  size of the ``receive_buf`` member of a :c:struct:`modem_cmux_dlci_config` instance, replace
+  it with ``CONFIG_MODEM_CMUX_MTU``.
+
 .. zephyr-keep-sorted-stop
 
 Modules

drivers/modem/Kconfig.cellular

@@ -32,6 +32,26 @@ config MODEM_CELLULAR
 
 if MODEM_CELLULAR
 
+config MODEM_CELLULAR_DEFAULT_CMUX_MTU_127
+	bool
+	default y if (DT_HAS_QUECTEL_BG95_ENABLED || DT_HAS_QUECTEL_EG25_G_ENABLED || \
+		      DT_HAS_SIMCOM_SIM7080_ENABLED || DT_HAS_U_BLOX_SARA_R4_ENABLED || \
+		      DT_HAS_U_BLOX_SARA_R5_ENABLED || DT_HAS_SWIR_HL7800_ENABLED || \
+		      DT_HAS_TELIT_ME910G1_ENABLED || DT_HAS_TELIT_ME310G1_ENABLED || \
+		      DT_HAS_SQN_GM02S_ENABLED || DT_HAS_QUECTEL_EG800Q_ENABLED || \
+		      DT_HAS_SIMCOM_A76XX_ENABLED)
+	help
+	  Use the default MTU size of 127 bytes for the CMUX module on certain modems.
+	  This must match the AT+CMUX commands in the modem_cellular driver.
+
+config MODEM_CELLULAR_CMUX_MTU
+	int "CMUX MTU size in bytes"
+	default 127 if MODEM_CELLULAR_DEFAULT_CMUX_MTU_127
+	default 31
+	help
+	  Maximum Transmission Unit (MTU) size for the CMUX module.
+	  Linux ldattach defaults to 127 bytes, 3GPP TS 27.010 to 31.
+
 config MODEM_CELLULAR_APN
 	string "APN"
 	default "internet"

drivers/modem/modem_cellular.c

@@ -93,18 +93,18 @@ struct modem_cellular_data {
 
 	/* CMUX */
 	struct modem_cmux cmux;
-	uint8_t cmux_receive_buf[CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE];
-	uint8_t cmux_transmit_buf[CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE];
+	uint8_t cmux_receive_buf[CONFIG_MODEM_CELLULAR_CMUX_MTU + 7];
+	uint8_t cmux_transmit_buf[CONFIG_MODEM_CELLULAR_CMUX_MTU + 22];
 
 	struct modem_cmux_dlci dlci1;
 	struct modem_cmux_dlci dlci2;
 	struct modem_pipe *dlci1_pipe;
 	struct modem_pipe *dlci2_pipe;
 	/* Points to dlci2_pipe or NULL. Used for shutdown script if not NULL */
 	struct modem_pipe *cmd_pipe;
-	uint8_t dlci1_receive_buf[CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE];
+	uint8_t dlci1_receive_buf[CONFIG_MODEM_CELLULAR_CMUX_MTU];
 	/* DLCI 2 is only used for chat scripts. */
-	uint8_t dlci2_receive_buf[CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE];
+	uint8_t dlci2_receive_buf[CONFIG_MODEM_CELLULAR_CMUX_MTU];
 
 	/* Modem chat */
 	struct modem_chat chat;
@@ -1868,6 +1868,7 @@ static int modem_cellular_init(const struct device *dev)
 			.receive_buf_size = ARRAY_SIZE(data->cmux_receive_buf),
 			.transmit_buf = data->cmux_transmit_buf,
 			.transmit_buf_size = ARRAY_SIZE(data->cmux_transmit_buf),
+			.mtu = CONFIG_MODEM_CELLULAR_CMUX_MTU,
 		};
 
 		modem_cmux_init(&data->cmux, &cmux_config);

include/zephyr/modem/cmux.h

@@ -151,12 +151,13 @@ struct modem_cmux {
 	uint16_t receive_buf_size;
 	uint16_t receive_buf_len;
 
-	uint8_t work_buf[CONFIG_MODEM_CMUX_WORK_BUFFER_SIZE];
-
 	/* Transmit buffer */
 	struct ring_buf transmit_rb;
 	struct k_mutex transmit_rb_lock;
 
+	/* Maximum Transmission Unit */
+	uint16_t mtu;
+
 	/* Received frame */
 	struct modem_cmux_frame frame;
 	uint8_t frame_header[5];
@@ -192,12 +193,31 @@ struct modem_cmux_config {
 	void *user_data;
 	/** Receive buffer */
 	uint8_t *receive_buf;
-	/** Size of receive buffer in bytes [127, ...] */
+	/**
+	 * Size of receive buffer in bytes.
+	 *
+	 * Minimum and recommended size is the MTU size + 7 which is
+	 * the maximum CMUX frame size.
+	 */
 	uint16_t receive_buf_size;
 	/** Transmit buffer */
 	uint8_t *transmit_buf;
-	/** Size of transmit buffer in bytes [149, ...] */
+	/**
+	 * Size of transmit buffer in bytes.
+	 *
+	 * Minimum and recommended size is the MTU size + 22 which is
+	 * the maximum CMUX frame size plus the reserved CMUX command
+	 * frame used for the control channel.
+	 */
 	uint16_t transmit_buf_size;
+	/**
+	 * Maximun Transmission Unit (MTU)
+	 *
+	 * Minimum size is 15 which is required for the control
+	 * channel. Recommended size is 127. Theoretical maximum size
+	 * is 16384.
+	 */
+	uint16_t mtu;
 };
 
 /**
@@ -215,7 +235,12 @@ struct modem_cmux_dlci_config {
 	uint8_t dlci_address;
 	/** Receive buffer used by pipe */
 	uint8_t *receive_buf;
-	/** Size of receive buffer used by pipe [127, ...] */
+	/**
+	 * Size of receive buffer in bytes.
+	 *
+	 * Minimum size is the MTU size which is the maximum amount
+	 * of data which can be stored in a single CMUX data frame.
+	 */
 	uint16_t receive_buf_size;
 };
 

subsys/modem/Kconfig

@@ -29,36 +29,6 @@ config MODEM_CMUX
 
 if MODEM_CMUX
 
-config MODEM_CMUX_DEFAULT_MTU_127
-	bool
-	default y if (DT_HAS_QUECTEL_BG95_ENABLED || DT_HAS_QUECTEL_EG25_G_ENABLED || \
-		      DT_HAS_SIMCOM_SIM7080_ENABLED || DT_HAS_U_BLOX_SARA_R4_ENABLED || \
-		      DT_HAS_U_BLOX_SARA_R5_ENABLED || DT_HAS_SWIR_HL7800_ENABLED || \
-		      DT_HAS_TELIT_ME910G1_ENABLED || DT_HAS_TELIT_ME310G1_ENABLED || \
-		      DT_HAS_SQN_GM02S_ENABLED || DT_HAS_QUECTEL_EG800Q_ENABLED || \
-		      DT_HAS_SIMCOM_A76XX_ENABLED)
-	help
-	  Use the default MTU size of 127 bytes for the CMUX module on certain modems.
-	  This must match the AT+CMUX commands in the modem_cellular driver.
-
-config MODEM_CMUX_MTU
-	int "CMUX MTU size in bytes"
-	range 16 1500
-	default 127 if MODEM_CMUX_DEFAULT_MTU_127
-	default 31
-	help
-	  Maximum Transmission Unit (MTU) size for the CMUX module.
-	  Linux ldattach defaults to 127 bytes, 3GPP TS 27.010 to 31.
-
-config MODEM_CMUX_WORK_BUFFER_SIZE
-	int "CMUX module work buffer size in bytes"
-	range 23 1507
-	default 134 if MODEM_CMUX_DEFAULT_MTU_127
-	default 38
-	help
-	  Size of the work buffer used by the CMUX module.
-	  Recommended size is MODEM_CMUX_MTU + 7 (CMUX header size).
-
 module = MODEM_CMUX
 module-str = modem_cmux
 source "subsys/logging/Kconfig.template.log_config"

subsys/modem/modem_cmux.c

@@ -27,6 +27,12 @@ LOG_MODULE_REGISTER(modem_cmux, CONFIG_MODEM_CMUX_LOG_LEVEL);
 #define MODEM_CMUX_CMD_FRAME_SIZE_MAX		(MODEM_CMUX_FRAME_SIZE_MAX + \
 						 MODEM_CMUX_CMD_DATA_SIZE_MAX)
 
+#define MODEM_CMUX_MTU_MIN_SIZE			(15)
+#define MODEM_CMUX_MTU_MAX_SIZE			(16384)
+#define MODEM_CMUX_RECEIVE_BUF_RESERVED		(MODEM_CMUX_FRAME_SIZE_MAX)
+#define MODEM_CMUX_TRANSMIT_BUF_RESERVED	(MODEM_CMUX_FRAME_SIZE_MAX + \
+						 MODEM_CMUX_CMD_FRAME_SIZE_MAX)
+
 #define MODEM_CMUX_T1_TIMEOUT			(K_MSEC(330))
 #define MODEM_CMUX_T2_TIMEOUT			(K_MSEC(660))
 
@@ -274,7 +280,7 @@ static uint16_t modem_cmux_transmit_frame(struct modem_cmux *cmux,
 
 	space = ring_buf_space_get(&cmux->transmit_rb) - MODEM_CMUX_FRAME_SIZE_MAX;
 	data_len = MIN(space, frame->data_len);
-	data_len = MIN(data_len, CONFIG_MODEM_CMUX_MTU);
+	data_len = MIN(data_len, cmux->mtu);
 
 	/* SOF */
 	buf[0] = 0xF9;
@@ -798,7 +804,7 @@ static void modem_cmux_process_received_byte(struct modem_cmux *cmux, uint8_t by
 			break;
 		}
 
-		if (cmux->frame.data_len > CONFIG_MODEM_CMUX_MTU) {
+		if (cmux->frame.data_len > cmux->mtu) {
 			LOG_ERR("Too large frame");
 			cmux->receive_state = MODEM_CMUX_RECEIVE_STATE_DROP;
 			break;
@@ -823,7 +829,7 @@ static void modem_cmux_process_received_byte(struct modem_cmux *cmux, uint8_t by
 		/* Get last 8 bits of data length */
 		cmux->frame.data_len |= ((uint16_t)byte) << 7;
 
-		if (cmux->frame.data_len > CONFIG_MODEM_CMUX_MTU) {
+		if (cmux->frame.data_len > cmux->mtu) {
 			LOG_ERR("Too large frame");
 			cmux->receive_state = MODEM_CMUX_RECEIVE_STATE_DROP;
 			break;
@@ -915,9 +921,19 @@ static void modem_cmux_receive_handler(struct k_work *item)
 	struct k_work_delayable *dwork = k_work_delayable_from_work(item);
 	struct modem_cmux *cmux = CONTAINER_OF(dwork, struct modem_cmux, receive_work);
 	int ret;
+	uint8_t *buf;
+	uint16_t buf_len;
+
+	/*
+	 * Use remaining data receive buffer as frame receive buffer. The frame
+	 * receive buffer will be overwritten by data one byte at a time as it
+	 * is processed.
+	 */
+	buf = cmux->receive_buf + cmux->receive_buf_len;
+	buf_len = cmux->receive_buf_size - cmux->receive_buf_len;
 
 	/* Receive data from pipe */
-	ret = modem_pipe_receive(cmux->pipe, cmux->work_buf, sizeof(cmux->work_buf));
+	ret = modem_pipe_receive(cmux->pipe, buf, buf_len);
 	if (ret < 1) {
 		if (ret < 0) {
 			LOG_ERR("Pipe receiving error: %d", ret);
@@ -927,7 +943,7 @@ static void modem_cmux_receive_handler(struct k_work *item)
 
 	/* Process received data */
 	for (int i = 0; i < ret; i++) {
-		modem_cmux_process_received_byte(cmux, cmux->work_buf[i]);
+		modem_cmux_process_received_byte(cmux, buf[i]);
 	}
 
 	/* Reschedule received work */
@@ -1251,18 +1267,20 @@ void modem_cmux_init(struct modem_cmux *cmux, const struct modem_cmux_config *co
 {
 	__ASSERT_NO_MSG(cmux != NULL);
 	__ASSERT_NO_MSG(config != NULL);
+	__ASSERT_NO_MSG(config->mtu >= MODEM_CMUX_MTU_MIN_SIZE);
+	__ASSERT_NO_MSG(config->mtu <= MODEM_CMUX_MTU_MAX_SIZE);
 	__ASSERT_NO_MSG(config->receive_buf != NULL);
-	__ASSERT_NO_MSG(config->receive_buf_size >=
-			(CONFIG_MODEM_CMUX_MTU + MODEM_CMUX_FRAME_SIZE_MAX));
+	__ASSERT_NO_MSG(config->receive_buf_size >= config->mtu + MODEM_CMUX_FRAME_SIZE_MAX);
 	__ASSERT_NO_MSG(config->transmit_buf != NULL);
 	__ASSERT_NO_MSG(config->transmit_buf_size >=
-			(CONFIG_MODEM_CMUX_MTU + MODEM_CMUX_FRAME_SIZE_MAX));
+		(config->mtu + MODEM_CMUX_FRAME_SIZE_MAX + MODEM_CMUX_CMD_FRAME_SIZE_MAX));
 
 	memset(cmux, 0x00, sizeof(*cmux));
 	cmux->callback = config->callback;
 	cmux->user_data = config->user_data;
 	cmux->receive_buf = config->receive_buf;
 	cmux->receive_buf_size = config->receive_buf_size;
+	cmux->mtu = config->mtu;
 	sys_slist_init(&cmux->dlcis);
 	cmux->state = MODEM_CMUX_STATE_DISCONNECTED;
 	ring_buf_init(&cmux->transmit_rb, config->transmit_buf_size, config->transmit_buf);

tests/subsys/modem/modem_cmux/prj.conf

@@ -5,6 +5,5 @@ CONFIG_NO_OPTIMIZATIONS=y
 
 CONFIG_MODEM_MODULES=y
 CONFIG_MODEM_CMUX=y
-CONFIG_MODEM_CMUX_MTU=64
 
 CONFIG_ZTEST=y

tests/subsys/modem/modem_cmux/src/main.c

@@ -27,6 +27,7 @@
 #define EVENT_CMUX_DLCI1_CLOSED		BIT(7)
 #define EVENT_CMUX_DLCI2_CLOSED		BIT(8)
 #define EVENT_CMUX_DISCONNECTED		BIT(9)
+#define TEST_CMUX_MTU_SIZE		64
 #define CMUX_BASIC_HRD_SMALL_SIZE	6
 #define CMUX_BASIC_HRD_LARGE_SIZE	7
 
@@ -280,6 +281,7 @@ static void *test_modem_cmux_setup(void)
 		.receive_buf_size = sizeof(cmux_receive_buf),
 		.transmit_buf = cmux_transmit_buf,
 		.transmit_buf_size = ARRAY_SIZE(cmux_transmit_buf),
+		.mtu = TEST_CMUX_MTU_SIZE,
 	};
 
 	modem_cmux_init(&cmux, &cmux_config);
@@ -875,14 +877,14 @@ ZTEST(modem_cmux, test_modem_cmux_split_large_data)
 
 	ret = modem_pipe_transmit(dlci2_pipe, cmux_frame_data_large,
 				  sizeof(cmux_frame_data_large));
-	zassert_true(ret == CONFIG_MODEM_CMUX_MTU, "Failed to split large data %d", ret);
+	zassert_true(ret == TEST_CMUX_MTU_SIZE, "Failed to split large data %d", ret);
 
 	events = k_event_wait(&cmux_event, EVENT_CMUX_DLCI2_TRANSMIT_IDLE, false, K_MSEC(200));
 	zassert_equal(events, EVENT_CMUX_DLCI2_TRANSMIT_IDLE,
 		      "Transmit idle event not received for DLCI2 pipe");
 
 	ret = modem_backend_mock_get(&bus_mock, buffer2, sizeof(buffer2));
-	zassert_true(ret == CONFIG_MODEM_CMUX_MTU + CMUX_BASIC_HRD_SMALL_SIZE,
+	zassert_true(ret == TEST_CMUX_MTU_SIZE + CMUX_BASIC_HRD_SMALL_SIZE,
 		     "Incorrect number of bytes transmitted %d", ret);
 }
 

tests/subsys/modem/modem_cmux_pair/prj.conf

@@ -5,6 +5,5 @@ CONFIG_NO_OPTIMIZATIONS=y
 
 CONFIG_MODEM_MODULES=y
 CONFIG_MODEM_CMUX=y
-CONFIG_MODEM_CMUX_MTU=64
 
 CONFIG_ZTEST=y

tests/subsys/modem/modem_cmux_pair/src/main.c

@@ -21,6 +21,8 @@
 #define EVENT_CMUX_DLCI1_RX_DATA BIT(6)
 #define EVENT_CMUX_DLCI2_RX_DATA BIT(7)
 
+#define TEST_CMUX_MTU_SIZE 64
+
 /* CMUX DTE variables */
 static struct modem_cmux cmux_dte;
 static uint8_t cmux_receive_buf[127];
@@ -221,6 +223,7 @@ static void cmux_dte_init(void)
 		.receive_buf_size = sizeof(cmux_receive_buf),
 		.transmit_buf = cmux_transmit_buf,
 		.transmit_buf_size = ARRAY_SIZE(cmux_transmit_buf),
+		.mtu = TEST_CMUX_MTU_SIZE,
 	};
 
 	const struct modem_backend_mock_config bus_mock_config = {
@@ -263,6 +266,7 @@ static void cmux_dce_init(void)
 		.receive_buf_size = sizeof(cmux_receive_buf_dce),
 		.transmit_buf = cmux_transmit_buf_dce,
 		.transmit_buf_size = ARRAY_SIZE(cmux_transmit_buf_dce),
+		.mtu = TEST_CMUX_MTU_SIZE,
 	};
 
 	const struct modem_backend_mock_config bus_mock_config = {