| Message ID | 20191104124844.8112-1-unixmania@gmail.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | package/modem-manager: use libqmi and libmbim if they are selected | expand |
Hey Carlos, > > If we have a cnfiguration like this > > BR2_PACKAGE_MODEM_MANAGER=y > # BR2_PACKAGE_MODEM_MANAGER_LIBMBIM is not set > BR2_PACKAGE_MODEM_MANAGER_LIBQMI=y > [...] > BR2_PACKAGE_LIBMBIM=y > > then libqmi is configured with --enable-mbim-qmux and requires libmbim > so ModemManager must be configured --with-mbim otherwise it fails to > link due to missing libmbim symbols required by libqmi: > > qmi-endpoint-mbim.c:(.text+0x158): undefined reference to `mbim_device_close_finish' > Wouldn't this Libs.private fix in libqmi solve this specific build problem? See http://lists.busybox.net/pipermail/buildroot/2019-October/264787.html > Prevent this kind of error by using a simpler approach: > > - Always enable MBIM support if libmbim is selected > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and add a legacy option that > selects BR2_PACKAGE_LIBMBIM > - Always enable QMI support if libqmi is selected > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBQMI and add a legacy option that > selects BR2_PACKAGE_LIBQMI > - Update the help text explaining how to enable MBIM and QMI > It is true that the current setup may lead to a strange configuration where ModemManager may be built with only QMI support and no explicit MBIM support while at the same time libqmi is built with MBIM support. That would not be any problem per se, it would just be weird. I'm not totally sure how these settings are usually preferred in buildroot, truth be told. Maybe someone with more experience in buildroot itself could suggest how to best handle this? Is it better to have a setting in the ModemManager package to enable/disable features explicitly? Or is it better to implicitly enable/disable those features based on whether some other packages are also included in the build? I guess the question would be whether we want to have a non-QMI non-MBIM capable ModemManager installed at the same time as libqmi+qmicli/libmbim+mbimcli. I have absolutely no idea whether this is a real usecase for anyone or not, truth be told. If it is (or may be) a real usecase, then I don't think this patch should go in.
On Mon, Nov 4, 2019 at 10:03 AM Aleksander Morgado <aleksander@aleksander.es> wrote: > > Hey Carlos, > > > > > If we have a cnfiguration like this > > > > BR2_PACKAGE_MODEM_MANAGER=y > > # BR2_PACKAGE_MODEM_MANAGER_LIBMBIM is not set > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI=y > > [...] > > BR2_PACKAGE_LIBMBIM=y > > > > then libqmi is configured with --enable-mbim-qmux and requires libmbim > > so ModemManager must be configured --with-mbim otherwise it fails to > > link due to missing libmbim symbols required by libqmi: > > > > qmi-endpoint-mbim.c:(.text+0x158): undefined reference to `mbim_device_close_finish' > > > > Wouldn't this Libs.private fix in libqmi solve this specific build > problem? See http://lists.busybox.net/pipermail/buildroot/2019-October/264787.html Yes, but it passed below my radar due to ETOOMUCHMAIL. :-) > > Prevent this kind of error by using a simpler approach: > > > > - Always enable MBIM support if libmbim is selected > > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and add a legacy option that > > selects BR2_PACKAGE_LIBMBIM > > - Always enable QMI support if libqmi is selected > > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBQMI and add a legacy option that > > selects BR2_PACKAGE_LIBQMI > > - Update the help text explaining how to enable MBIM and QMI > > > > It is true that the current setup may lead to a strange configuration > where ModemManager may be built with only QMI support and no explicit > MBIM support while at the same time libqmi is built with MBIM support. > That would not be any problem per se, it would just be weird. > > I'm not totally sure how these settings are usually preferred in > buildroot, truth be told. Maybe someone with more experience in > buildroot itself could suggest how to best handle this? Is it better > to have a setting in the ModemManager package to enable/disable > features explicitly? Or is it better to implicitly enable/disable > those features based on whether some other packages are also included > in the build? I guess the question would be whether we want to have a > non-QMI non-MBIM capable ModemManager installed at the same time as > libqmi+qmicli/libmbim+mbimcli. I have absolutely no idea whether this > is a real usecase for anyone or not, truth be told. If it is (or may > be) a real usecase, then I don't think this patch should go in. > > -- > Aleksander > https://aleksander.es The main client of ModemManager is NetworkManager, which always selects MBIM and QMI support, so I don't believe it would make much difference, in practice.
Hey Carlos! > > > If we have a cnfiguration like this > > > > > > BR2_PACKAGE_MODEM_MANAGER=y > > > # BR2_PACKAGE_MODEM_MANAGER_LIBMBIM is not set > > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI=y > > > [...] > > > BR2_PACKAGE_LIBMBIM=y > > > > > > then libqmi is configured with --enable-mbim-qmux and requires libmbim > > > so ModemManager must be configured --with-mbim otherwise it fails to > > > link due to missing libmbim symbols required by libqmi: > > > > > > qmi-endpoint-mbim.c:(.text+0x158): undefined reference to `mbim_device_close_finish' > > > > > > > Wouldn't this Libs.private fix in libqmi solve this specific build > > problem? See http://lists.busybox.net/pipermail/buildroot/2019-October/264787.html > > Yes, but it passed below my radar due to ETOOMUCHMAIL. :-) > > > > Prevent this kind of error by using a simpler approach: > > > > > > - Always enable MBIM support if libmbim is selected > > > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and add a legacy option that > > > selects BR2_PACKAGE_LIBMBIM > > > - Always enable QMI support if libqmi is selected > > > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBQMI and add a legacy option that > > > selects BR2_PACKAGE_LIBQMI > > > - Update the help text explaining how to enable MBIM and QMI > > > > > > > It is true that the current setup may lead to a strange configuration > > where ModemManager may be built with only QMI support and no explicit > > MBIM support while at the same time libqmi is built with MBIM support. > > That would not be any problem per se, it would just be weird. > > > > I'm not totally sure how these settings are usually preferred in > > buildroot, truth be told. Maybe someone with more experience in > > buildroot itself could suggest how to best handle this? Is it better > > to have a setting in the ModemManager package to enable/disable > > features explicitly? Or is it better to implicitly enable/disable > > those features based on whether some other packages are also included > > in the build? I guess the question would be whether we want to have a > > non-QMI non-MBIM capable ModemManager installed at the same time as > > libqmi+qmicli/libmbim+mbimcli. I have absolutely no idea whether this > > is a real usecase for anyone or not, truth be told. If it is (or may > > be) a real usecase, then I don't think this patch should go in. > > The main client of ModemManager is NetworkManager, which always > selects MBIM and QMI support, so I don't believe it would make much > difference, in practice. > If you ask me, the NetworkManager package should not select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM or BR2_PACKAGE_MODEM_MANAGER_LIBQMI. The ModemManager API that NetworkManager uses doesn't in any way change based on whether QMI/MBIM support is enabled, it's really orthogonal. @Yegor Yefremov what's your take on this?
Hi Aleksander, Carlos, On Mon, Nov 4, 2019 at 3:36 PM Aleksander Morgado <aleksander@aleksander.es> wrote: > > Hey Carlos! > > > > > If we have a cnfiguration like this > > > > > > > > BR2_PACKAGE_MODEM_MANAGER=y > > > > # BR2_PACKAGE_MODEM_MANAGER_LIBMBIM is not set > > > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI=y > > > > [...] > > > > BR2_PACKAGE_LIBMBIM=y > > > > > > > > then libqmi is configured with --enable-mbim-qmux and requires libmbim > > > > so ModemManager must be configured --with-mbim otherwise it fails to > > > > link due to missing libmbim symbols required by libqmi: > > > > > > > > qmi-endpoint-mbim.c:(.text+0x158): undefined reference to `mbim_device_close_finish' > > > > > > > > > > Wouldn't this Libs.private fix in libqmi solve this specific build > > > problem? See http://lists.busybox.net/pipermail/buildroot/2019-October/264787.html > > > > Yes, but it passed below my radar due to ETOOMUCHMAIL. :-) > > > > > > Prevent this kind of error by using a simpler approach: > > > > > > > > - Always enable MBIM support if libmbim is selected > > > > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and add a legacy option that > > > > selects BR2_PACKAGE_LIBMBIM > > > > - Always enable QMI support if libqmi is selected > > > > - Drop BR2_PACKAGE_MODEM_MANAGER_LIBQMI and add a legacy option that > > > > selects BR2_PACKAGE_LIBQMI > > > > - Update the help text explaining how to enable MBIM and QMI > > > > > > > > > > It is true that the current setup may lead to a strange configuration > > > where ModemManager may be built with only QMI support and no explicit > > > MBIM support while at the same time libqmi is built with MBIM support. > > > That would not be any problem per se, it would just be weird. > > > > > > I'm not totally sure how these settings are usually preferred in > > > buildroot, truth be told. Maybe someone with more experience in > > > buildroot itself could suggest how to best handle this? Is it better > > > to have a setting in the ModemManager package to enable/disable > > > features explicitly? Or is it better to implicitly enable/disable > > > those features based on whether some other packages are also included > > > in the build? I guess the question would be whether we want to have a > > > non-QMI non-MBIM capable ModemManager installed at the same time as > > > libqmi+qmicli/libmbim+mbimcli. I have absolutely no idea whether this > > > is a real usecase for anyone or not, truth be told. If it is (or may > > > be) a real usecase, then I don't think this patch should go in. > > > > The main client of ModemManager is NetworkManager, which always > > selects MBIM and QMI support, so I don't believe it would make much > > difference, in practice. > > > > If you ask me, the NetworkManager package should not select > BR2_PACKAGE_MODEM_MANAGER_LIBMBIM or BR2_PACKAGE_MODEM_MANAGER_LIBQMI. > The ModemManager API that NetworkManager uses doesn't in any way > change based on whether QMI/MBIM support is enabled, it's really > orthogonal. > @Yegor Yefremov what's your take on this? I don't have a strong opinion here. I'm always using MM with both QMI and MBIM libraries. As you already need glib2 for MM, one won't waste too much disk space. Hence I would opt for MM to always enable both libraries. This would simplify both MM and NM configuration. Yegor
Hello, Adding our Config.in.legacy expert Arnout. On Mon, 4 Nov 2019 09:48:44 -0300 unixmania@gmail.com wrote: > diff --git a/Config.in.legacy b/Config.in.legacy > index fcb10b1291..3336ef85d1 100644 > --- a/Config.in.legacy > +++ b/Config.in.legacy > @@ -146,6 +146,22 @@ endif > > comment "Legacy options removed in 2019.11" > > +config BR2_PACKAGE_MODEM_MANAGER_LIBMBIM > + bool "BR2_PACKAGE_MODEM_MANAGER_LIBMBIM has been removed" > + select BR2_LEGACY > + select BR2_PACKAGE_LIBMBIM > + help > + Select BR2_PACKAGE_LIBMBIM to enable support for MBIM protocol > + in ModemManager > + > +config BR2_PACKAGE_MODEM_MANAGER_LIBQMI > + bool "BR2_PACKAGE_MODEM_MANAGER_LIBQMI has been removed" > + select BR2_LEGACY > + select BR2_PACKAGE_LIBQMI > + help > + Select BR2_PACKAGE_LIBQMI to enable support for QMI protocol > + in ModemManager I think in this specific case, we don't need Config.in.legacy handling. Indeed, a full .config that had BR2_PACKAGE_MODEM_MANAGER_LIBQMI enabled will already have BR2_PACKAGE_LIBQMI, and since the .mk file now relies on BR2_PACKAGE_LIBQMI to enable libqmi support in modem-manager, I think we're good. Of course, that won't work for defconfigs, but I believe updating with a defconfig is not really guaranteed to give the right result. Arnout ? Other than that, patch looks good to me. No need to resend, let's just wait to see what Arnout says about the Config.in.legacy handling. Best regards, Thomas
On 04/11/2019 22:05, Thomas Petazzoni wrote: > Hello, > > Adding our Config.in.legacy expert Arnout. > > On Mon, 4 Nov 2019 09:48:44 -0300 > unixmania@gmail.com wrote: > >> diff --git a/Config.in.legacy b/Config.in.legacy >> index fcb10b1291..3336ef85d1 100644 >> --- a/Config.in.legacy >> +++ b/Config.in.legacy >> @@ -146,6 +146,22 @@ endif >> >> comment "Legacy options removed in 2019.11" >> >> +config BR2_PACKAGE_MODEM_MANAGER_LIBMBIM >> + bool "BR2_PACKAGE_MODEM_MANAGER_LIBMBIM has been removed" >> + select BR2_LEGACY >> + select BR2_PACKAGE_LIBMBIM >> + help >> + Select BR2_PACKAGE_LIBMBIM to enable support for MBIM protocol >> + in ModemManager >> + >> +config BR2_PACKAGE_MODEM_MANAGER_LIBQMI >> + bool "BR2_PACKAGE_MODEM_MANAGER_LIBQMI has been removed" >> + select BR2_LEGACY >> + select BR2_PACKAGE_LIBQMI >> + help >> + Select BR2_PACKAGE_LIBQMI to enable support for QMI protocol >> + in ModemManager > > I think in this specific case, we don't need Config.in.legacy handling. > Indeed, a full .config that had BR2_PACKAGE_MODEM_MANAGER_LIBQMI > enabled will already have BR2_PACKAGE_LIBQMI, and since the .mk file > now relies on BR2_PACKAGE_LIBQMI to enable libqmi support in > modem-manager, I think we're good. > > Of course, that won't work for defconfigs, but I believe updating with > a defconfig is not really guaranteed to give the right result. Arnout ? In my point of view, indeed it isn't, but this isn't documented anywhere I think. So indeed, I agree that no legacy handling is needed here. It will annoy many users (because they have to go and disable it in the legacy menu), while being of no benefit for most of them. Regards, Arnout > Other than that, patch looks good to me. No need to resend, let's just > wait to see what Arnout says about the Config.in.legacy handling. > > Best regards, > > Thomas >
On Wed, Nov 6, 2019 at 10:12 AM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 04/11/2019 22:05, Thomas Petazzoni wrote: > > Hello, > > > > Adding our Config.in.legacy expert Arnout. > > > > On Mon, 4 Nov 2019 09:48:44 -0300 > > unixmania@gmail.com wrote: > > > >> diff --git a/Config.in.legacy b/Config.in.legacy > >> index fcb10b1291..3336ef85d1 100644 > >> --- a/Config.in.legacy > >> +++ b/Config.in.legacy > >> @@ -146,6 +146,22 @@ endif > >> > >> comment "Legacy options removed in 2019.11" > >> > >> +config BR2_PACKAGE_MODEM_MANAGER_LIBMBIM > >> + bool "BR2_PACKAGE_MODEM_MANAGER_LIBMBIM has been removed" > >> + select BR2_LEGACY > >> + select BR2_PACKAGE_LIBMBIM > >> + help > >> + Select BR2_PACKAGE_LIBMBIM to enable support for MBIM protocol > >> + in ModemManager > >> + > >> +config BR2_PACKAGE_MODEM_MANAGER_LIBQMI > >> + bool "BR2_PACKAGE_MODEM_MANAGER_LIBQMI has been removed" > >> + select BR2_LEGACY > >> + select BR2_PACKAGE_LIBQMI > >> + help > >> + Select BR2_PACKAGE_LIBQMI to enable support for QMI protocol > >> + in ModemManager > > > > I think in this specific case, we don't need Config.in.legacy handling. > > Indeed, a full .config that had BR2_PACKAGE_MODEM_MANAGER_LIBQMI > > enabled will already have BR2_PACKAGE_LIBQMI, and since the .mk file > > now relies on BR2_PACKAGE_LIBQMI to enable libqmi support in > > modem-manager, I think we're good. > > > > Of course, that won't work for defconfigs, but I believe updating with > > a defconfig is not really guaranteed to give the right result. Arnout ? > > In my point of view, indeed it isn't, but this isn't documented anywhere I think. > > So indeed, I agree that no legacy handling is needed here. It will annoy many > users (because they have to go and disable it in the legacy menu), while being > of no benefit for most of them. > > Regards, > Arnout > > > > Other than that, patch looks good to me. No need to resend, let's just > > wait to see what Arnout says about the Config.in.legacy handling. > > > > Best regards, > > > > Thomas > > I suggest to apply https://patchwork.ozlabs.org/patch/1186596/ to solve the build problem, then we can decide if modem-manager and network-manager need additional changes.
Hello Carlos, Aleksander, Yegor, On Mon, 4 Nov 2019 09:48:44 -0300 unixmania@gmail.com wrote: > If we have a cnfiguration like this > > BR2_PACKAGE_MODEM_MANAGER=y > # BR2_PACKAGE_MODEM_MANAGER_LIBMBIM is not set > BR2_PACKAGE_MODEM_MANAGER_LIBQMI=y > [...] > BR2_PACKAGE_LIBMBIM=y I had a new look at this. We need a new iteration of this patch, since the build issue doesn't exist anymore. However, I would suggest to take a simpler approach: - Keep the BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and BR2_PACKAGE_MODEM_MANAGER_LIBQMI options. - Change modem-manager.mk to test BR2_PACKAGE_LIBMBIM and BR2_PACKAGE_QMI instead of BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and BR2_PACKAGE_MODEM_MANAGER_LIBQMI. Also, as was pointed out during the discussion, it would be good to change the network-manager package to not force the modem-manager MBIM/QMI support when modem-manager support is enabled. I.e, remove the following lines: select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM select BR2_PACKAGE_MODEM_MANAGER_LIBQMI from package/network-manager/Config.in Could one of you have a look ? Thanks a lot! Thomas
Hey Thomas, > > However, I would suggest to take a simpler approach: > > - Keep the BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > BR2_PACKAGE_MODEM_MANAGER_LIBQMI options. > > - Change modem-manager.mk to test BR2_PACKAGE_LIBMBIM and > BR2_PACKAGE_QMI instead of BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > BR2_PACKAGE_MODEM_MANAGER_LIBQMI. > I'm not sure what you mean with the above paragraph. What do you mean with e.g. "testing" BR2_PACKAGE_LIBMBIM instead of BR2_PACKAGE_:MODEM_MANAGER_LIBMBIM? > Also, as was pointed out during the discussion, it would be good to > change the network-manager package to not force the modem-manager > MBIM/QMI support when modem-manager support is enabled. I.e, remove the > following lines: > > select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM > select BR2_PACKAGE_MODEM_MANAGER_LIBQMI > > from package/network-manager/Config.in > I agree with this change in NM, I'll send a patch for that. But is it just removing those lines? Ideally, MBIM and QMI (and QMI over MBIM) support should be "the default" when building MM, and the users can disable them explicitly if not required. I'm not sure how to handle that in buildroot, truth be told.
Hello Aleksander, On Mon, 20 Apr 2020 11:11:45 +0200 Aleksander Morgado <aleksander@aleksander.es> wrote: > > - Change modem-manager.mk to test BR2_PACKAGE_LIBMBIM and > > BR2_PACKAGE_QMI instead of BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI. > > > > I'm not sure what you mean with the above paragraph. What do you mean > with e.g. "testing" BR2_PACKAGE_LIBMBIM instead of > BR2_PACKAGE_:MODEM_MANAGER_LIBMBIM? I mean this: diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk index 75fc5811db..6b4e9b6250 100644 --- a/package/modem-manager/modem-manager.mk +++ b/package/modem-manager/modem-manager.mk @@ -13,7 +13,7 @@ MODEM_MANAGER_DEPENDENCIES = host-pkgconf libglib2 $(TARGET_NLS_DEPENDENCIES) MODEM_MANAGER_INSTALL_STAGING = YES MODEM_MANAGER_CONF_OPTS = --disable-more-warnings -ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBQMI),y) +ifeq ($(BR2_PACKAGE_LIBQMI),y) MODEM_MANAGER_DEPENDENCIES += libqmi MODEM_MANAGER_CONF_OPTS += --with-qmi else @@ -27,7 +27,7 @@ else MODEM_MANAGER_CONF_OPTS += --without-udev endif -ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBMBIM),y) +ifeq ($(BR2_PACKAGE_LIBMBIM),y) MODEM_MANAGER_DEPENDENCIES += libmbim MODEM_MANAGER_CONF_OPTS += --with-mbim else > > Also, as was pointed out during the discussion, it would be good to > > change the network-manager package to not force the modem-manager > > MBIM/QMI support when modem-manager support is enabled. I.e, remove the > > following lines: > > > > select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM > > select BR2_PACKAGE_MODEM_MANAGER_LIBQMI > > > > from package/network-manager/Config.in > > > > I agree with this change in NM, I'll send a patch for that. But is it > just removing those lines? Yes. > Ideally, MBIM and QMI (and QMI over MBIM) support should be "the > default" when building MM, and the users can disable them explicitly > if not required. I'm not sure how to handle that in buildroot, truth > be told. Normally, the Buildroot policy is to make all features optional in the upstream software really optional in Buildroot as well. I.e, if modem-manager can build and be useful with qmi/mbim support, then we shouldn't force to build the modem-manager support with qmi/mbim support. Is modem-manager completely useless without qmi/mbim support ? Does network-manager absolutely requires qmi/mbim support in modem-manager ? If not, then qmi/mbim should be optional, and not force-selected by network-manager when enabling modem-manager support. Thanks! Best regards, Thomas
Hey Thomas, > > > - Change modem-manager.mk to test BR2_PACKAGE_LIBMBIM and > > > BR2_PACKAGE_QMI instead of BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI. > > > > > > > I'm not sure what you mean with the above paragraph. What do you mean > > with e.g. "testing" BR2_PACKAGE_LIBMBIM instead of > > BR2_PACKAGE_:MODEM_MANAGER_LIBMBIM? > > I mean this: > > diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk > index 75fc5811db..6b4e9b6250 100644 > --- a/package/modem-manager/modem-manager.mk > +++ b/package/modem-manager/modem-manager.mk > @@ -13,7 +13,7 @@ MODEM_MANAGER_DEPENDENCIES = host-pkgconf libglib2 $(TARGET_NLS_DEPENDENCIES) > MODEM_MANAGER_INSTALL_STAGING = YES > MODEM_MANAGER_CONF_OPTS = --disable-more-warnings > > -ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBQMI),y) > +ifeq ($(BR2_PACKAGE_LIBQMI),y) > MODEM_MANAGER_DEPENDENCIES += libqmi > MODEM_MANAGER_CONF_OPTS += --with-qmi > else > @@ -27,7 +27,7 @@ else > MODEM_MANAGER_CONF_OPTS += --without-udev > endif > > -ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBMBIM),y) > +ifeq ($(BR2_PACKAGE_LIBMBIM),y) > MODEM_MANAGER_DEPENDENCIES += libmbim > MODEM_MANAGER_CONF_OPTS += --with-mbim > else > > > > Also, as was pointed out during the discussion, it would be good to > > > change the network-manager package to not force the modem-manager > > > MBIM/QMI support when modem-manager support is enabled. I.e, remove the > > > following lines: > > > > > > select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM > > > select BR2_PACKAGE_MODEM_MANAGER_LIBQMI > > > > > > from package/network-manager/Config.in > > > > > > > I agree with this change in NM, I'll send a patch for that. But is it > > just removing those lines? > > Yes. > > > Ideally, MBIM and QMI (and QMI over MBIM) support should be "the > > default" when building MM, and the users can disable them explicitly > > if not required. I'm not sure how to handle that in buildroot, truth > > be told. > > Normally, the Buildroot policy is to make all features optional in the > upstream software really optional in Buildroot as well. I.e, if > modem-manager can build and be useful with qmi/mbim support, then we > shouldn't force to build the modem-manager support with qmi/mbim > support. > > Is modem-manager completely useless without qmi/mbim support ? Does > network-manager absolutely requires qmi/mbim support in modem-manager ? > > If not, then qmi/mbim should be optional, and not force-selected by > network-manager when enabling modem-manager support. > Better late than never! I've sent several patches addressing the above, let me know if there's any problem with them. Cheers!
Thomas, Aleksander, All, On 2020-04-19 23:00 +0200, Thomas Petazzoni spake thusly: > Hello Carlos, Aleksander, Yegor, > > On Mon, 4 Nov 2019 09:48:44 -0300 > unixmania@gmail.com wrote: > > > If we have a cnfiguration like this > > > > BR2_PACKAGE_MODEM_MANAGER=y > > # BR2_PACKAGE_MODEM_MANAGER_LIBMBIM is not set > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI=y > > [...] > > BR2_PACKAGE_LIBMBIM=y > > I had a new look at this. We need a new iteration of this patch, since > the build issue doesn't exist anymore. > > However, I would suggest to take a simpler approach: > > - Keep the BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > BR2_PACKAGE_MODEM_MANAGER_LIBQMI options. > > - Change modem-manager.mk to test BR2_PACKAGE_LIBMBIM and > BR2_PACKAGE_QMI instead of BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > BR2_PACKAGE_MODEM_MANAGER_LIBQMI. Sorry, but I disagree here: a user who elects to not enable 'MBIM support' in modem-manager, but otherwise has libmbim enabled (for whatever reason) will in fact get a modem-mamanger that has support for MBIM. This is definitely counter-intuitive. Ditto libqmi. So I would rather agree with the original patch from Carlos (except for the legacy handling, which is indeed not needed). > Also, as was pointed out during the discussion, it would be good to > change the network-manager package to not force the modem-manager > MBIM/QMI support when modem-manager support is enabled. I.e, remove the > following lines: > > select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM > select BR2_PACKAGE_MODEM_MANAGER_LIBQMI > > from package/network-manager/Config.in I do agree with that too, which must be done in a separate patch. Regards, Yann E. MORIN. > Could one of you have a look ? > > Thanks a lot! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hey Yann, > > > BR2_PACKAGE_MODEM_MANAGER=y > > > # BR2_PACKAGE_MODEM_MANAGER_LIBMBIM is not set > > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI=y > > > [...] > > > BR2_PACKAGE_LIBMBIM=y > > > > I had a new look at this. We need a new iteration of this patch, since > > the build issue doesn't exist anymore. > > > > However, I would suggest to take a simpler approach: > > > > - Keep the BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI options. > > > > - Change modem-manager.mk to test BR2_PACKAGE_LIBMBIM and > > BR2_PACKAGE_QMI instead of BR2_PACKAGE_MODEM_MANAGER_LIBMBIM and > > BR2_PACKAGE_MODEM_MANAGER_LIBQMI. > > Sorry, but I disagree here: a user who elects to not enable 'MBIM > support' in modem-manager, but otherwise has libmbim enabled (for > whatever reason) will in fact get a modem-mamanger that has support for > MBIM. This is definitely counter-intuitive. > > Ditto libqmi. > > So I would rather agree with the original patch from Carlos (except for > the legacy handling, which is indeed not needed). > I have very mixed feelings here. I do agree with you in the sense that yes, if the software allows the possibility of having the system with libmbim installed *and* ModemManager *without MBIM* support installed, then why not allow that? It even gets more complicated than that actually, because libqmi may also be compiled with or without MBIM support (for the QMI over MBIM transport), so if we're up to adding lots of config options, we could even handle that as well, otherwise why even bother with the MM config options. In MM 1.14 there is also going to be the possibility to select which plugins to build and install, if users only need some. Really, we can make it as complex as we want build config wise. But, on the other hand, I truly fail to see a case where a user may want to have libmbim installed and MM built without MBIM support (I haven't seen such a use case myself, and believe me I've seen lots of use cases!). Or both libmbim and libqmi installed but MM only built with QMI support and not MBIM. And what if MM wants to have MBIM and QMI support but libqmi hasn't been built with MBIM support? I think we would be making it complex for the sake of complexity, instead of just making it simple. The fact that MM was built with the possibility to disable MBIM support was really thought for the case where the system isn't going to have ever a MBIM device so you can "save" the kernel driver, the libmbim library, and still have MM to handle e.g. AT modems. Simplifying the config I think is good, and I kind of liked Thomas' suggestion here. If I have to select among both points of view, I would really prefer simplifying, because it really just makes sense to have MBIM support in both libqmi and ModemManager if libmbim is installed, and same for libqmi. If we don't do that, I'm sure we'll get lots of users selecting the wrong configurations and having to debug why the hell MM isn't processing the QMI modem if libqmi and qmicli can happily talk to the device, for example. Removing config options isn't always bad, sometimes it's actually better in the long run. This is just my opinion, and as I said, is not a strong one :D > > Also, as was pointed out during the discussion, it would be good to > > change the network-manager package to not force the modem-manager > > MBIM/QMI support when modem-manager support is enabled. I.e, remove the > > following lines: > > > > select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM > > select BR2_PACKAGE_MODEM_MANAGER_LIBQMI > > > > from package/network-manager/Config.in > > I do agree with that too, which must be done in a separate patch. > There's a separate patch doing that already, it's also in the mailing list. Cheers!
diff --git a/Config.in.legacy b/Config.in.legacy index fcb10b1291..3336ef85d1 100644 --- a/Config.in.legacy +++ b/Config.in.legacy @@ -146,6 +146,22 @@ endif comment "Legacy options removed in 2019.11" +config BR2_PACKAGE_MODEM_MANAGER_LIBMBIM + bool "BR2_PACKAGE_MODEM_MANAGER_LIBMBIM has been removed" + select BR2_LEGACY + select BR2_PACKAGE_LIBMBIM + help + Select BR2_PACKAGE_LIBMBIM to enable support for MBIM protocol + in ModemManager + +config BR2_PACKAGE_MODEM_MANAGER_LIBQMI + bool "BR2_PACKAGE_MODEM_MANAGER_LIBQMI has been removed" + select BR2_LEGACY + select BR2_PACKAGE_LIBQMI + help + Select BR2_PACKAGE_LIBQMI to enable support for QMI protocol + in ModemManager + config BR2_PACKAGE_ALLJOYN bool "alljoyn was removed" select BR2_LEGACY diff --git a/package/modem-manager/Config.in b/package/modem-manager/Config.in index c4c723776d..e7987ad1e6 100644 --- a/package/modem-manager/Config.in +++ b/package/modem-manager/Config.in @@ -10,22 +10,11 @@ config BR2_PACKAGE_MODEM_MANAGER ModemManager is a DBus-activated daemon which controls mobile broadband (2G/3G/4G) devices and connections. - http://www.freedesktop.org/wiki/Software/ModemManager/ - -if BR2_PACKAGE_MODEM_MANAGER + Select BR2_PACKAGE_LIBMBIM to enable support for MBIM protocol -config BR2_PACKAGE_MODEM_MANAGER_LIBMBIM - bool "MBIM support" - select BR2_PACKAGE_LIBMBIM - help - This option enables support for MBIM protocol + Select BR2_PACKAGE_LIBQMI to enable support for QMI protocol -config BR2_PACKAGE_MODEM_MANAGER_LIBQMI - bool "QMI support" - select BR2_PACKAGE_LIBQMI - help - This option enables support for QMI protocol -endif + http://www.freedesktop.org/wiki/Software/ModemManager/ comment "modemmanager needs a toolchain w/ wchar, threads" depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk index dde841b80a..554d1a235e 100644 --- a/package/modem-manager/modem-manager.mk +++ b/package/modem-manager/modem-manager.mk @@ -13,7 +13,7 @@ MODEM_MANAGER_DEPENDENCIES = host-pkgconf libglib2 $(TARGET_NLS_DEPENDENCIES) MODEM_MANAGER_INSTALL_STAGING = YES MODEM_MANAGER_CONF_OPTS = --disable-more-warnings -ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBQMI),y) +ifeq ($(BR2_PACKAGE_LIBQMI),y) MODEM_MANAGER_DEPENDENCIES += libqmi MODEM_MANAGER_CONF_OPTS += --with-qmi else @@ -27,7 +27,7 @@ else MODEM_MANAGER_CONF_OPTS += --without-udev endif -ifeq ($(BR2_PACKAGE_MODEM_MANAGER_LIBMBIM),y) +ifeq ($(BR2_PACKAGE_LIBMBIM),y) MODEM_MANAGER_DEPENDENCIES += libmbim MODEM_MANAGER_CONF_OPTS += --with-mbim else diff --git a/package/network-manager/Config.in b/package/network-manager/Config.in index 1257e08e6f..79ad161f31 100644 --- a/package/network-manager/Config.in +++ b/package/network-manager/Config.in @@ -37,10 +37,11 @@ config BR2_PACKAGE_NETWORK_MANAGER_TUI config BR2_PACKAGE_NETWORK_MANAGER_MODEM_MANAGER bool "modem-manager support" select BR2_PACKAGE_MODEM_MANAGER - select BR2_PACKAGE_MODEM_MANAGER_LIBMBIM - select BR2_PACKAGE_MODEM_MANAGER_LIBQMI + select BR2_PACKAGE_LIBMBIM + select BR2_PACKAGE_LIBQMI help - This option enables support for ModemManager + This option enables support for ModemManager with support for + MBIM and QMI protocols config BR2_PACKAGE_NETWORK_MANAGER_PPPD bool "pppd support"