diff mbox series

[1/1] package/modem-manager: enable qcom-soc if libqrtr-glib enabled

Message ID 20210404184126.974098-1-petr.vorel@gmail.com
State New
Headers show
Series [1/1] package/modem-manager: enable qcom-soc if libqrtr-glib enabled | expand

Commit Message

Petr Vorel April 4, 2021, 6:41 p.m. UTC
This plugin makes sense only for Qualcomm MSM phone.
Although there is currently no such device defconfig in Buildroot yet,
it makes sense to prepare modem-manager for it.

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
 package/modem-manager/modem-manager.mk | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Thomas Petazzoni April 5, 2021, 9:02 a.m. UTC | #1
On Sun,  4 Apr 2021 20:41:26 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:

> This plugin makes sense only for Qualcomm MSM phone.
> Although there is currently no such device defconfig in Buildroot yet,
> it makes sense to prepare modem-manager for it.
> 
> Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> ---
>  package/modem-manager/modem-manager.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk
> index 2cc45d266b..d62d13cf45 100644
> --- a/package/modem-manager/modem-manager.mk
> +++ b/package/modem-manager/modem-manager.mk
> @@ -34,6 +34,12 @@ else
>  MODEM_MANAGER_CONF_OPTS += --without-mbim
>  endif
>  
> +ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
> +MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc

Isn't that missing a MODEM_MANAGER_DEPENDENCIES += libqrtr-glib line ?
Or is it really a runtime dependency ?

Thomas
Petr Vorel April 5, 2021, 10:17 a.m. UTC | #2
> On Sun,  4 Apr 2021 20:41:26 +0200
> Petr Vorel <petr.vorel@gmail.com> wrote:

> > This plugin makes sense only for Qualcomm MSM phone.
> > Although there is currently no such device defconfig in Buildroot yet,
> > it makes sense to prepare modem-manager for it.

> > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > ---
> >  package/modem-manager/modem-manager.mk | 6 ++++++
> >  1 file changed, 6 insertions(+)

> > diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk
> > index 2cc45d266b..d62d13cf45 100644
> > --- a/package/modem-manager/modem-manager.mk
> > +++ b/package/modem-manager/modem-manager.mk
> > @@ -34,6 +34,12 @@ else
> >  MODEM_MANAGER_CONF_OPTS += --without-mbim
> >  endif

> > +ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
> > +MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc

> Isn't that missing a MODEM_MANAGER_DEPENDENCIES += libqrtr-glib line ?
> Or is it really a runtime dependency ?
error: Couldn't find libqmi-glib >= 1.29.3
when building locally.
=> you're right it probably requires MODEM_MANAGER_DEPENDENCIES += libqrtr-glib
I'll test it and probably sent v2.
Thanks for catching error!

Kind regards,
Petr

> Thomas
Petr Vorel April 5, 2021, 1:44 p.m. UTC | #3
> On Sun,  4 Apr 2021 20:41:26 +0200
> Petr Vorel <petr.vorel@gmail.com> wrote:

> > This plugin makes sense only for Qualcomm MSM phone.
> > Although there is currently no such device defconfig in Buildroot yet,
> > it makes sense to prepare modem-manager for it.

> > Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
> > ---
> >  package/modem-manager/modem-manager.mk | 6 ++++++
> >  1 file changed, 6 insertions(+)

> > diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk
> > index 2cc45d266b..d62d13cf45 100644
> > --- a/package/modem-manager/modem-manager.mk
> > +++ b/package/modem-manager/modem-manager.mk
> > @@ -34,6 +34,12 @@ else
> >  MODEM_MANAGER_CONF_OPTS += --without-mbim
> >  endif

> > +ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
> > +MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc

> Isn't that missing a MODEM_MANAGER_DEPENDENCIES += libqrtr-glib line ?
> Or is it really a runtime dependency ?
Actually verified locally on buildroot, it works as is.
(no need for MODEM_MANAGER_DEPENDENCIES += libqrtr-glib).

Thus I've updated patchwork status to NEW.

Kind regards,
Petr

> Thomas
Thomas Petazzoni April 5, 2021, 2:36 p.m. UTC | #4
On Mon, 5 Apr 2021 15:44:13 +0200
Petr Vorel <petr.vorel@gmail.com> wrote:

> > > +ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
> > > +MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc  
> 
> > Isn't that missing a MODEM_MANAGER_DEPENDENCIES += libqrtr-glib line ?
> > Or is it really a runtime dependency ?  
> Actually verified locally on buildroot, it works as is.
> (no need for MODEM_MANAGER_DEPENDENCIES += libqrtr-glib).
> 
> Thus I've updated patchwork status to NEW.

How can this work? Does it dlopen() the libqrtr-glib library at
runtime? This deserves some explanation as we would normally imagine
that to be a build-time dependency.

Thomas
Aleksander Morgado April 5, 2021, 2:45 p.m. UTC | #5
On Mon, 5 Apr 2021, 16:36 Thomas Petazzoni, <thomas.petazzoni@bootlin.com>
wrote:

> On Mon, 5 Apr 2021 15:44:13 +0200
> Petr Vorel <petr.vorel@gmail.com> wrote:
>
> > > > +ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
> > > > +MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc
> >
> > > Isn't that missing a MODEM_MANAGER_DEPENDENCIES += libqrtr-glib line ?
> > > Or is it really a runtime dependency ?
> > Actually verified locally on buildroot, it works as is.
> > (no need for MODEM_MANAGER_DEPENDENCIES += libqrtr-glib).
> >
> > Thus I've updated patchwork status to NEW.
>
> How can this work? Does it dlopen() the libqrtr-glib library at
> runtime? This deserves some explanation as we would normally imagine
> that to be a build-time dependency.
>


It's probably too soon to add libqrtr-glib as a dependency of MM, the QRTR
support is not even in MM git master yet (likely this week).

libqrtr-glib is right now only a dependency of libqmi, I think we should
focus on that. Let's add the MM build-time dependency once there is a MM
release with QRTR support.

>
Petr Vorel April 5, 2021, 3:51 p.m. UTC | #6
Hi Thomas, Aleksander,

> On Mon, 5 Apr 2021, 16:36 Thomas Petazzoni, <thomas.petazzoni@bootlin.com>
> wrote:

> > On Mon, 5 Apr 2021 15:44:13 +0200
> > Petr Vorel <petr.vorel@gmail.com> wrote:

> > > > > +ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
> > > > > +MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc

> > > > Isn't that missing a MODEM_MANAGER_DEPENDENCIES += libqrtr-glib line ?
> > > > Or is it really a runtime dependency ?
> > > Actually verified locally on buildroot, it works as is.
> > > (no need for MODEM_MANAGER_DEPENDENCIES += libqrtr-glib).

> > > Thus I've updated patchwork status to NEW.

> > How can this work? Does it dlopen() the libqrtr-glib library at
> > runtime? This deserves some explanation as we would normally imagine
> > that to be a build-time dependency.



> It's probably too soon to add libqrtr-glib as a dependency of MM, the QRTR
> support is not even in MM git master yet (likely this week).

> libqrtr-glib is right now only a dependency of libqmi, I think we should
> focus on that. Let's add the MM build-time dependency once there is a MM
> release with QRTR support.

Thomas, sorry for confusion. Aleksander, please correct me if I'm wrong. Looking
at ModemManager sources (8fc60754 "qcom-soc: new plugin for Qualcomm SoCs") it
currently does not depend on libqrtr-glib at all (not sure if you want use
libqrtr-glib dependency later only for qcom-soc for for something else in
ModemManager).

I based decision for --enable-plugin-qcom-soc on BR2_PACKAGE_LIBQRTR_GLIB,
because if one wants Qualcomm IPC Router protocol helper library, he probably
wants also qcom-soc plugin. And even nobody is now using Buildroot for Qualcomm
phones, it can change in the future.

Kind regards,
Petr
Aleksander Morgado April 6, 2021, 7:29 a.m. UTC | #7
Hey,

> > > > > > +ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
> > > > > > +MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc
>
> > > > > Isn't that missing a MODEM_MANAGER_DEPENDENCIES += libqrtr-glib line ?
> > > > > Or is it really a runtime dependency ?
> > > > Actually verified locally on buildroot, it works as is.
> > > > (no need for MODEM_MANAGER_DEPENDENCIES += libqrtr-glib).
>
> > > > Thus I've updated patchwork status to NEW.
>
> > > How can this work? Does it dlopen() the libqrtr-glib library at
> > > runtime? This deserves some explanation as we would normally imagine
> > > that to be a build-time dependency.
>
>
>
> > It's probably too soon to add libqrtr-glib as a dependency of MM, the QRTR
> > support is not even in MM git master yet (likely this week).
>
> > libqrtr-glib is right now only a dependency of libqmi, I think we should
> > focus on that. Let's add the MM build-time dependency once there is a MM
> > release with QRTR support.
>
> Thomas, sorry for confusion. Aleksander, please correct me if I'm wrong. Looking
> at ModemManager sources (8fc60754 "qcom-soc: new plugin for Qualcomm SoCs") it
> currently does not depend on libqrtr-glib at all (not sure if you want use
> libqrtr-glib dependency later only for qcom-soc for for something else in
> ModemManager).
>

Yes, that's exactly what will happen. Once the QRTR support is
integrated, it will only be used in the qcom-soc plugin. If you don't
plan to build the qcom-soc plugin, you could build ModemManager
configuring --without-qrtr and/or disabling the QRTR support also in
libqmi.

> I based decision for --enable-plugin-qcom-soc on BR2_PACKAGE_LIBQRTR_GLIB,
> because if one wants Qualcomm IPC Router protocol helper library, he probably
> wants also qcom-soc plugin. And even nobody is now using Buildroot for Qualcomm
> phones, it can change in the future.

It isn't strictly necessary though, you could use the qcom-soc plugin
without QRTR, using RPMSG instead (as it is the case right now in MM
1.16). It really depends on the SoC you're targeting to use.
diff mbox series

Patch

diff --git a/package/modem-manager/modem-manager.mk b/package/modem-manager/modem-manager.mk
index 2cc45d266b..d62d13cf45 100644
--- a/package/modem-manager/modem-manager.mk
+++ b/package/modem-manager/modem-manager.mk
@@ -34,6 +34,12 @@  else
 MODEM_MANAGER_CONF_OPTS += --without-mbim
 endif
 
+ifeq ($(BR2_PACKAGE_LIBQRTR_GLIB),y)
+MODEM_MANAGER_CONF_OPTS += --enable-plugin-qcom-soc
+else
+MODEM_MANAGER_CONF_OPTS += --disable-plugin-qcom-soc
+endif
+
 define MODEM_MANAGER_INSTALL_INIT_SYSV
 	$(INSTALL) -m 0755 -D package/modem-manager/S44modem-manager \
 		$(TARGET_DIR)/etc/init.d/S44modem-manager