All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional
@ 2017-09-05 10:03 Aleksander Morgado
  2017-09-05 19:36 ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Aleksander Morgado @ 2017-09-05 10:03 UTC (permalink / raw)
  To: buildroot

Don't always build without udev, as qmi-firmware-update would be very
very limited in that case. Instead, make it optional: if there is udev
support in the setup, require libgudev and configure using --with-udev
explicitly; otherwise just --without-udev.

Also, add the qmi-over-mbim feature as optional, and require libmbim
if we're building with it enabled.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 package/libqmi/Config.in | 18 ++++++++++++++++++
 package/libqmi/libqmi.mk | 18 ++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/package/libqmi/Config.in b/package/libqmi/Config.in
index f1d111b7c..a536650b5 100644
--- a/package/libqmi/Config.in
+++ b/package/libqmi/Config.in
@@ -10,6 +10,24 @@ config BR2_PACKAGE_LIBQMI
 
 	  http://www.freedesktop.org/wiki/Software/libqmi/
 
+if BR2_PACKAGE_LIBQMI
+
+config BR2_PACKAGE_LIBQMI_UDEV
+	bool "qmi-firmware-update udev support"
+	depends on BR2_PACKAGE_HAS_UDEV
+	select BR2_PACKAGE_LIBGUDEV
+	help
+	  This option enables udev support in the qmi-firmware-update tool
+
+config BR2_PACKAGE_LIBQMI_MBIM_QMUX
+	bool "QMI-over-MBIM support"
+	select BR2_PACKAGE_LIBMBIM
+	help
+	  This option enables support to use the QMI protocol over MBIM
+	  for modems with MBIM_SERVICE_QMI capabilities
+
+endif
+
 comment "libqmi needs a toolchain w/ wchar, threads"
 	depends on BR2_USE_MMU
 	depends on !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/libqmi/libqmi.mk b/package/libqmi/libqmi.mk
index 917265f4b..129fd0fb6 100644
--- a/package/libqmi/libqmi.mk
+++ b/package/libqmi/libqmi.mk
@@ -15,7 +15,21 @@ LIBQMI_AUTORECONF = YES
 
 LIBQMI_DEPENDENCIES = libglib2
 
-# we don't want -Werror and disable gudev Gobject bindings
-LIBQMI_CONF_OPTS = --enable-more-warnings=no --without-udev
+# we don't want -Werror
+LIBQMI_CONF_OPTS = --enable-more-warnings=no
+
+ifeq ($(BR2_PACKAGE_LIBQMI_UDEV),y)
+LIBQMI_DEPENDENCIES += libgudev
+LIBQMI_CONF_OPTS += --with-udev
+else
+LIBQMI_CONF_OPTS += --without-udev
+endif
+
+ifeq ($(BR2_PACKAGE_LIBQMI_MBIM_QMUX),y)
+LIBQMI_DEPENDENCIES += libmbim
+LIBQMI_CONF_OPTS += --enable-mbim-qmux
+else
+LIBQMI_CONF_OPTS += --disable-mbim-qmux
+endif
 
 $(eval $(autotools-package))
-- 
2.13.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional
  2017-09-05 10:03 [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional Aleksander Morgado
@ 2017-09-05 19:36 ` Thomas Petazzoni
  2017-09-05 19:45   ` Aleksander Morgado
  2017-09-06  8:38   ` Aleksander Morgado
  0 siblings, 2 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2017-09-05 19:36 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue,  5 Sep 2017 12:03:27 +0200, Aleksander Morgado wrote:
> Don't always build without udev, as qmi-firmware-update would be very
> very limited in that case. Instead, make it optional: if there is udev
> support in the setup, require libgudev and configure using --with-udev
> explicitly; otherwise just --without-udev.
> 
> Also, add the qmi-over-mbim feature as optional, and require libmbim
> if we're building with it enabled.
> 
> Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
> ---
>  package/libqmi/Config.in | 18 ++++++++++++++++++
>  package/libqmi/libqmi.mk | 18 ++++++++++++++++--
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/package/libqmi/Config.in b/package/libqmi/Config.in
> index f1d111b7c..a536650b5 100644
> --- a/package/libqmi/Config.in
> +++ b/package/libqmi/Config.in
> @@ -10,6 +10,24 @@ config BR2_PACKAGE_LIBQMI
>  
>  	  http://www.freedesktop.org/wiki/Software/libqmi/
>  
> +if BR2_PACKAGE_LIBQMI
> +
> +config BR2_PACKAGE_LIBQMI_UDEV
> +	bool "qmi-firmware-update udev support"
> +	depends on BR2_PACKAGE_HAS_UDEV
> +	select BR2_PACKAGE_LIBGUDEV

libgudev has plenty of other dependencies that you need to propagate
here.

> +	help
> +	  This option enables udev support in the qmi-firmware-update tool
> +
> +config BR2_PACKAGE_LIBQMI_MBIM_QMUX
> +	bool "QMI-over-MBIM support"
> +	select BR2_PACKAGE_LIBMBIM

... and libmbim also has plenty of dependencies that you need to
propagate here, including BR2_PACKAGE_HAS_UDEV.

All in all, isn't it simpler to get rid of those options, and simply do:

ifeq ($(BR2_PACKAGE_LIBGUDEV),y)
... enable support
else
... disable support
endif

ifeq ($(BR2_PACKAGE_LIBMBIM),y)
... enable support
else
... disable support
endif

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional
  2017-09-05 19:36 ` Thomas Petazzoni
@ 2017-09-05 19:45   ` Aleksander Morgado
  2017-09-06  6:54     ` Thomas Petazzoni
  2017-09-06  8:38   ` Aleksander Morgado
  1 sibling, 1 reply; 6+ messages in thread
From: Aleksander Morgado @ 2017-09-05 19:45 UTC (permalink / raw)
  To: buildroot

Hey

>
> On Tue,  5 Sep 2017 12:03:27 +0200, Aleksander Morgado wrote:
>> Don't always build without udev, as qmi-firmware-update would be very
>> very limited in that case. Instead, make it optional: if there is udev
>> support in the setup, require libgudev and configure using --with-udev
>> explicitly; otherwise just --without-udev.
>>
>> Also, add the qmi-over-mbim feature as optional, and require libmbim
>> if we're building with it enabled.
>>
>> Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
>> ---
>>  package/libqmi/Config.in | 18 ++++++++++++++++++
>>  package/libqmi/libqmi.mk | 18 ++++++++++++++++--
>>  2 files changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/package/libqmi/Config.in b/package/libqmi/Config.in
>> index f1d111b7c..a536650b5 100644
>> --- a/package/libqmi/Config.in
>> +++ b/package/libqmi/Config.in
>> @@ -10,6 +10,24 @@ config BR2_PACKAGE_LIBQMI
>>
>>         http://www.freedesktop.org/wiki/Software/libqmi/
>>
>> +if BR2_PACKAGE_LIBQMI
>> +
>> +config BR2_PACKAGE_LIBQMI_UDEV
>> +     bool "qmi-firmware-update udev support"
>> +     depends on BR2_PACKAGE_HAS_UDEV
>> +     select BR2_PACKAGE_LIBGUDEV
>
> libgudev has plenty of other dependencies that you need to propagate
> here.
>
>> +     help
>> +       This option enables udev support in the qmi-firmware-update tool
>> +
>> +config BR2_PACKAGE_LIBQMI_MBIM_QMUX
>> +     bool "QMI-over-MBIM support"
>> +     select BR2_PACKAGE_LIBMBIM
>
> ... and libmbim also has plenty of dependencies that you need to
> propagate here, including BR2_PACKAGE_HAS_UDEV.
>

Oh, I assumed that was somehow automatic... Does this mean that if the
deps of a library are updated at some point in time, all the apps
depending on the library also need to get updated to add the new dep?

> All in all, isn't it simpler to get rid of those options, and simply do:
>
> ifeq ($(BR2_PACKAGE_LIBGUDEV),y)
> ... enable support
> else
> ... disable support
> endif
>
> ifeq ($(BR2_PACKAGE_LIBMBIM),y)
> ... enable support
> else
> ... disable support
> endif
>

Yes, probably that's a much simpler option. Will suggest a new patch.

Thanks for reviewing!

-- 
Aleksander
https://aleksander.es

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional
  2017-09-05 19:45   ` Aleksander Morgado
@ 2017-09-06  6:54     ` Thomas Petazzoni
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2017-09-06  6:54 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 5 Sep 2017 21:45:19 +0200, Aleksander Morgado wrote:

> > ... and libmbim also has plenty of dependencies that you need to
> > propagate here, including BR2_PACKAGE_HAS_UDEV.
> >  
> 
> Oh, I assumed that was somehow automatic... Does this mean that if the
> deps of a library are updated at some point in time, all the apps
> depending on the library also need to get updated to add the new dep?

Yes, exactly. When we review patches adding new dependencies to an
existing package, we are always careful that those new dependencies are
propagated.

> Yes, probably that's a much simpler option. Will suggest a new patch.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional
  2017-09-05 19:36 ` Thomas Petazzoni
  2017-09-05 19:45   ` Aleksander Morgado
@ 2017-09-06  8:38   ` Aleksander Morgado
  2017-09-06 11:32     ` Thomas Petazzoni
  1 sibling, 1 reply; 6+ messages in thread
From: Aleksander Morgado @ 2017-09-06  8:38 UTC (permalink / raw)
  To: buildroot

>
> All in all, isn't it simpler to get rid of those options, and simply do:
>
> ifeq ($(BR2_PACKAGE_LIBGUDEV),y)
> ... enable support

When doing this, should I also include a direct dependency on
libgudev, so that libgudev is built before libqmi always?
LIBQMI_DEPENDENCIES += libgudev

> else
> ... disable support
> endif
>



-- 
Aleksander
https://aleksander.es

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional
  2017-09-06  8:38   ` Aleksander Morgado
@ 2017-09-06 11:32     ` Thomas Petazzoni
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2017-09-06 11:32 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 6 Sep 2017 10:38:25 +0200, Aleksander Morgado wrote:

> > All in all, isn't it simpler to get rid of those options, and simply do:
> >
> > ifeq ($(BR2_PACKAGE_LIBGUDEV),y)
> > ... enable support  
> 
> When doing this, should I also include a direct dependency on
> libgudev, so that libgudev is built before libqmi always?
> LIBQMI_DEPENDENCIES += libgudev

Well, if you don't do this, libgudev is not guaranteed to be built
before libqmi, so you would get a build failure, right ?

The canonical way to express optional dependencies is:

ifeq ($(BR2_PACKAGE_FOO),y)
BAR_DEPENDENCIES += foo
BAR_CONF_OPTS += --enable-foo
else
BAR_CONF_OPTS += --disable-foo
endif

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-09-06 11:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05 10:03 [Buildroot] [PATCH] libqmi: udev and qmi-over-mbim are optional Aleksander Morgado
2017-09-05 19:36 ` Thomas Petazzoni
2017-09-05 19:45   ` Aleksander Morgado
2017-09-06  6:54     ` Thomas Petazzoni
2017-09-06  8:38   ` Aleksander Morgado
2017-09-06 11:32     ` Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.