[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[unixmania at gmail.com]

From: Carlos Santos <unixmania@gmail.com>

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'

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

Fixes:
  http://autobuild.buildroot.net/results/9c6b8ec2b9cc31f1ab460532c378731ab455210c/

Signed-off-by: Carlos Santos <unixmania@gmail.com>
---
 Config.in.legacy                       | 16 ++++++++++++++++
 package/modem-manager/Config.in        | 17 +++--------------
 package/modem-manager/modem-manager.mk |  4 ++--
 package/network-manager/Config.in      |  7 ++++---
 4 files changed, 25 insertions(+), 19 deletions(-)

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"
-- 
2.18.1

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Aleksander Morgado]

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.

-- 
Aleksander
https://aleksander.es

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Carlos Santos]

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.

-- 
Carlos Santos <unixmania@gmail.com>

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Aleksander Morgado]

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?

-- 
Aleksander
https://aleksander.es

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Yegor Yefremov]

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

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Thomas Petazzoni]

Hello,

Adding our Config.in.legacy expert Arnout.

On Mon,  4 Nov 2019 09:48:44 -0300
unixmania at 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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Arnout Vandecappelle]

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 at 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
> 

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Carlos Santos]

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 at 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.

-- 
Carlos Santos <unixmania@gmail.com>

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Thomas Petazzoni]

Hello Carlos, Aleksander, Yegor,

On Mon,  4 Nov 2019 09:48:44 -0300
unixmania at 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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Aleksander Morgado]

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.

-- 
Aleksander
https://aleksander.es

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Aleksander Morgado]

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!

-- 
Aleksander
https://aleksander.

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Yann E. MORIN]

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 at 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 at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] package/modem-manager: use libqmi and libmbim if they are selected

[Aleksander Morgado]

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!

-- 
Aleksander
https://aleksander.es