Skip to content

modem: cmux: Decouple modem cmux and modem cellular driver #93441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2025
Merged

modem: cmux: Decouple modem cmux and modem cellular driver #93441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions drivers/modem/Kconfig.cellular
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ config MODEM_CELLULAR
DT_HAS_QUECTEL_EG25_G_ENABLED || DT_HAS_NORDIC_NRF91_SLM_ENABLED || \
DT_HAS_SQN_GM02S_ENABLED || DT_HAS_U_BLOX_LARA_R6_ENABLED || \
DT_HAS_QUECTEL_EG800Q_ENABLED)
select MODEM_CMUX_DEFAULT_MTU_127 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
This driver uses the generic 3gpp AT commands, along
with the standard protocols CMUX and PPP, to configure
Expand Down
9 changes: 1 addition & 8 deletions subsys/modem/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,8 @@ 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.
Hint that the default MODEM_CMUX_MTU size should be 127 bytes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it's not just a hint? It's enforced by the CMUX module?

Copy link
Contributor Author

@bjarki-andreasen bjarki-andreasen Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its enforced by MODEM_CMUX_MTU :) The default is just a default, so this hint is telling CMUX that something in the system would prefer MTU be 127, but it is definitely still directly settable by the application

Copy link
Contributor Author

@bjarki-andreasen bjarki-andreasen Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking a bit closer, what does MODEM_CMUX_MTU do? It seems like an artificial limit, given the actual limit of CMUX frame sizes is MODEM_CMUX_WORK_BUFFER_SIZE, where is it used favorably to MODEM_CMUX_WORK_BUFFER_SIZE?

Copy link
Contributor Author

@bjarki-andreasen bjarki-andreasen Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, we have no way to configure the MTU used by CMUX, major oversight on my end when creating the API...

The struct modem_cmux_config should be updated to include the MTU, this way, it becomes instance specific and needs no global config. On top of that, part of the receive buffer should be reserved for the work buffer. These two changes remove all hardcoded configs from the CMUX implementation :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good. FYI @SeppoTakalo @MarkusLassila

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMUX frame sizes were last worked on PR #87115
And it is configurable by Kconfig.
Surely it can be a runtime configurable, but does it really make much difference? You have selected the modem already in the build time.

Some modems use hard-coded MTU for the CMUX. Some allow it to be configured in the AT command that enables the CMUX framing.
Even some clients hard-code the value. For example Linux ldattach uses 127.
3GPP default is 32.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking a bit closer, what does MODEM_CMUX_MTU do? It seems like an artificial limit

It is not an artificial limit, frames bigger than the MTU what the receiver can process, are dropped.

It is not directly related to how much we have buffer space. It depends on the remote end as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is not to make the MTU runtime configurable, but to make it instance specific instead a global property.


config MODEM_CMUX_MTU
int "CMUX MTU size in bytes"
Expand Down