Upgrade to ModemManager 1.16.8 #7
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Attempting to change the initial EPS bearer settings while in full functionality mode shouldn't happen, so make sure we don't attempt to do that if going into low power mode fails.
Don't go to next step and then check if we need to jump to the RF on step, jump right away.
There is no need to reload it on every settings update attempt; just load it once when the 3GPP interface is initialized, and re-use the loaded value on every new update attempt.
The class object definition should always be last in the file, following the interface definitions. The actual method implementations should be given before any other type system method (i.e. before even the _new() method).
The mm_base_modem_get_port_*() returns a full reference, we should use the mm_base_modem_peek_port_*() methods instead. Also, refactor a bit the logic because both ports are really configured in the same way, so just apply the same setup to both.
We will use one single method to apply port type hints, not a mix of
them:
* If AT^GETPORTMODE is supported, prefer its hints over any other
method.
* Otherwise, try to guess hints from USB interface descriptions.
* And if none of the plugin-specific hints are supported, we'll
default to applying generic port type hints from udev tags.
Once the hints have been applied by one of the methods above, the
fallback hint sequences are run:
* Flag the first cdc-wdm port as primary if no other port has been
flagged as primary.
* Flag the USB interface 0 as PPP if no other port type hint has
been set in any other port.
The logic applying all these procedures has been refactored so that we
have separate functions for each, which is much easier to read and
follow, even if it requires multiple iterations over the port probe
list.
Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/238
So don't re-process them in the generic modem when grabbing the port.
The numbers associated to each port mode given by the AT^GETPORTMODE response are not USB interface numbers, they are 'port numbers'. Moreover, these numbers may start either at 0 or at 1, depending on the firmware. The only reasonable way to parse this response is to just gather the order of all the port modes reported, and apply the modes to each serial port found in the system in the same order. Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/239
If multiple kernel device types inherit from MMKernerDevice, and those are compared against each other, the current logic returns TRUE if the G_OBJECT_TYPE of one of them is smaller than the other. This function is checking for equality, so returning FALSE is enough.
…s Output Do it earlier, before running the parser.
This is not just a rename of the rule, we also now avoid doing an explicit check on the port name as well, and we rely on subsystem checks only; i.e. the same logic applied for net ports. The port candidate rules already do a 'cdc-wdm*' device name check so it shouldn't be a big deal.
The ModemManager-names.h header file is generated on git builds, and shipped within the release tarball, so treat it as a maintainer managed file that should only be removed on 'maintainer-clean'. This allows us to avoid requiring xsltproc to build release tarballs.
From gtkdocize (gtk-doc) 1.33.0.
The .actions file is generated by newer gtk-doc releases, we just treat it as e.g. the .signals or .types files. This is also already fixed in upstream gtk-doc: https://gitlab.gnome.org/GNOME/gtk-doc/-/merge_requests/61
There should be no need to do an early check to filter out ports of the wrong subsystem. For the user of these methods it is irrelevant if the FALSE is returned because the port is of the wrong subsystem, or because the test wasn't added to be probed, or because the test actually failed. In other words, the ports where the test succeeded will only have succeeded if they are of the correct subsystem and if the test was actually executed successfully.
The rules to force ignoring certain network ports because the modem is using some specific drivers should definitely only be applied for the very known qmi_wwan and cdc_mbim drivers. If network ports for QMI or MBIM modems are exposed using different network drivers, don't ignore them.
Move the logic out of the base modem, and make it applicable only for MBIM modems.
…orts By default we provide the implementation for the cdc_mbim driver, where both control and net ports share the same USB interface.
No longer has to be bound to the USB subsystem.
Move the logic out of the base modem, and make it applicable only for QMI modems.
By default we provide the implementation for the qmi_wwan driver, where both control and net ports share the same USB interface.
No longer has to be bound to the USB subsystem.
It is no longer true that all QMI ports are exposed by the qmi_wwan driver and that all MBIM ports are exposed by the cdc_mbim driver. There are other generic setups that allow exposing these types of ports using different drivers, and usually we can also know the type of port in advance via other means. Therefore, allow adding udev port type hints for QMI and MBIM ports as well.
Back in Linux < 3.6 days, the cdc-wdm ports exposed by the QMI driver were flagged as owned by the 'usb' subsystem. That changed in 3.6 when the subsystem was renamed to 'usbmisc': https://mail.gnome.org/archives/networkmanager-list/2012-June/msg00125.html So, rename the port subsystem type enumn to 'usbmisc'.
This check is pointless given that we're not implementing API, if anything it should be an assert. Anyway, just get rid of it, so that we don't need to update it on every new subsystem we add as supported.
Use the new generic FCC unlock step instead of implementing it within the power up setup logic. (cherry picked from commit 76f24256174039a75926cdc0ce3a6b1f03ea10ee)
Use the new "DMS Foxconn Set FCC authentication" command to request the modem unlock during a power up operation. Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/373 (cherry picked from commit 9fdbbc6929cb61d4168e1696a94e5b2f07bc578e)
This reverts commit 2cb38c5.
This reverts commit da23f9c. The stable 1.16 branch does not build-depend on any libmbim git master version, so we should not report the QDU firmware update method as supported in MM at this point. It would not have been a big problem if we did expose the QDU firmware update method, because the logic in fwupd checks the MM and libmbim versions separately, but I think it's better to skip reporting QDU as supported ourselves for consistency with the minimum libmbim version we require.
This message is too specific, and there are a lot of cases where it wouldn't apply. E.g. when a MBIM module is stuck in low power mode due to some other reason (like hardware rfkill), this message would pop up to the users and completely confuse them. (cherry picked from commit 464c0d464c758387ac3d35f4e148db2c4368edba)
For now WWAN subsystem support has only been validated for PCI/MHI based devices and extra patches for USB based devices (qmi_wwan, cdc-mbim...) may be required, so do not consider WWAN ports being on USB bus. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> (cherry picked from commit c39d1d2fbc62dca759efcf8dc5b80c69a45269de)
Explicitly ignore the "wwan_dev" device as it is not associated with separate ports (which is what MM needs), but with the whole device instead. See https://lists.freedesktop.org/archives/modemmanager-devel/2021-May/008629.html (cherry picked from commit ac18b5ac5bf841cd3a619a9e07137c332558d6c4)
Recent changes in the WWAN framework in the kernel changed the WWAN port names from e.g. "wwan0p1AT" and "wwan0p2QMI" to "wwan0at0" and "wwan0qmi0" [1, 2]. This means that the udev rules no longer match since AT/QMI are now lower-case and no longer at the end of the name. However, recently additionally a "type" sysfs attribute was added for all WWAN ports [3], which makes it much more reliable to match the WWAN port names without relying on their exact device name. Add some additional udev rules to apply the port hints based on the "type" sysfs attributes. This fixes the port enumeration on newer Linux kernels that include the aforementioned commits. Note that we still need to keep the old udev rules for now since Linux 5.13 does not have the "type" attribute yet. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=392c26f7f133b9f09e5f58db1ce6ef4b3b4df49f [2]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f458709ff40b0d992fec496952f79c7820dd3fde [3]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=b3e22e10fdda8e7be3830289a4a63ae8b88d450c (cherry picked from commit 4282c8d843a144f20f0e1bd9f632751af7a5ff70)
FIREHOSE ports are used for firmware updates which are done independently from ModemManager, so we should ignore those ports. A similar change was originally made by Loic Poulain here: https://gitlab.freedesktop.org/loicpoulain/ModemManager/-/commit/32b0eb79cdbb37afe33abb72fd948be3cbaf6a2d (cherry picked from commit 43d705ab933ee021ff1c4c0ea5075db205be7a40)
The original logic in the MBIM modem would assume that if we had more than one network interface in the same modem, we could connect multiple data interfaces, each one with a different session. That logic is actually wrong, when using the master (non-multiplexed) network interface we should always use session id 0, which is the one holding all non-VLAN-tagged traffic. So, remove the logic that automatically creates new bearers with a different session id, as that is really wrong. (cherry picked from commit e5305498b7ddbc5e419cf51fc0c30aa6593a40d2)
(cherry picked from commit ce50d9c483639d7a7e62addab3eecba177c80a6a)
(cherry picked from commit fefd59e3ce26f8248f94d40f2fb1f68085ca36ee)
If we return a reference to the internal array in ensure_internal_supported_storages() and then we free the array unconditionally in the caller, the next time we call it, the array would be valid, but it would be fully empty. (cherry picked from commit dfeaec99ceb3ec5b6fd6ec7add1ce81978d68cfb)
(cherry picked from commit 059f13ffd33d6cf8c981119dd40e63e7abc2fe59)
The properties object stored in the context is being returned as task result; so we should make sure that object is no longer left in the context so that it's not freed twice. Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/395 (cherry picked from commit e1fbc1ccb74ba9c788d88c5f19fb89200a6478f6)
There are modems (e.g. Nokia, Thuraya, Iridium) which don't require or don't support power management, and therefore there is no way to either load or update the power status. In those modems we just assume ON is the current and only value (set in the skeleton during initialization) and so when we attempt to update the power state to ON during enabling, the logic should not break. Fix the logic by making sure the requested_power_setup() function pointers are only checked for validity after ensuring we're not already in the desired power state. Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/issues/398 (cherry picked from commit 63f3dcb2fe3dee42fe223751c5394a751e7ecc42)
The signal definition is:
<signal name="StateChanged">
<arg name="old" type="i" />
<arg name="new" type="i" />
<arg name="reason" type="u" />
</signal>
So the first two arguments need to be adjusted.
Without this change, the test was failing for me:
(/build/source/tools/tests/.libs/lt-test-stub:77030): GLib-GObject-CRITICAL **: 15:05:35.276: ../gobject/gsignal.c:3167: value for 'gint' parameter 0 for signal "state-changed" is of type 'guint'
(cherry picked from commit a8f086bfb3be65de133365b55595a9800ecaa681)
Will fail to build otherwise: mm-filter.c: In function 'mm_filter_port': mm-filter.c:245:13: warning: implicit declaration of function 'mm_dbg' [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-function-declaration-Wimplicit-function-declaration8;;] 245 | mm_dbg ("[filter] (%s/%s): XMM7360 TTY command port allowed", subsystem, name); | ^~~~~~ mm-filter.c:245:13: warning: nested extern declaration of 'mm_dbg' [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wnested-externs-Wnested-externs8;;] […] /nix/store/hy3lz2vfv9qq2v5jz9nzlx6mmiaq79rj-binutils-2.35.1/bin/ld: ModemManager-mm-filter.o: in function `mm_filter_port': /build/ModemManager-1.16.8/src/mm-filter.c:245: undefined reference to `mm_dbg' collect2: error: ld returned 1 exit status
Owner
|
Thanks for your PR. As you can see I haven't touched this project in a while, I just didn't have the need for working WWAN in the last months. Also I lost track of the current state of affairs regarding Linux support for XMM7360. There seems to be an official driver but only for 7560 and also ongoing efforts for improving the python version. At the moment I'm not sure whether it's worth following my approach (building a "native" ModemManager plugin). |
Author
|
Looking around a bit on the linux kernel mailing lists, there's a I didn't give any 5.14 rc a spin to see if it also exposes the 7360 modem, but will report back! |
Owner
|
Thanks for the pointer. I fiddled around a bit with 5.14 but without modification it doesn't support 7360. I added the 7360 to the list of supported PCI devices, compiled the module and at least it doesn't crash. I can't communicate with the exposed AT ports, though. And the driver assumes that one of the shared memory queue pairs directly speaks MBIM, which then can be used by e.g. ModemManager. But according to the original author of xmm7360-pci, the 7360 device doesn't expose MBIM. So at the moment I'm stuck at this point. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I merged in 1.16.8, and updated all references to logging functions to use the allowed ones.
Otherwise, loading the plugin fails with