libqmi: udev and qmi-over-mbim are optional
PATCH 1/1 libqmi: udev and qmi-over-mbim are optional
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.
--- package/libqmi/Config.in | 18 ++++++++++++++++++ package/libqmi/libqmi.mk | 18 ++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-)
package/libqmi/Config.in +18 -0
@@ -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
package/libqmi/libqmi.mk +16 -2
@@ -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))
Signature · 1 line
2.13.1
PATCH 1/1 libqmi: udev and qmi-over-mbim are optional
Hello,
Quoted 19 lines · Aleksander Morgado
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 <[email protected]>--- package/libqmi/Config.in | 18 ++++++++++++++++++ package/libqmi/libqmi.mk | 18 ++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-)package/libqmi/Config.in +0
-0
package/libqmi/Config.in config BR2_PACKAGE_LIBQMI · line 18
Quoted 9 lines
package/libqmi/Config.in +6
-0@@ -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.
package/libqmi/Config.in config BR2_PACKAGE_LIBQMI · line 24
Quoted 6 lines
package/libqmi/Config.in +6
-0+ 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
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 libqmi: udev and qmi-over-mbim are optional
Hey
Quoted 34 lines · Aleksander Morgado
Quoted 19 lines
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 <[email protected]>--- package/libqmi/Config.in | 18 ++++++++++++++++++ package/libqmi/libqmi.mk | 18 ++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-)package/libqmi/Config.in +0
-0package/libqmi/Config.in config BR2_PACKAGE_LIBQMI · line 18
Quoted 9 lines
package/libqmi/Config.in +6
-0@@ -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_LIBGUDEVlibgudev has plenty of other dependencies that you need to propagate here.
Quoted 10 lines
package/libqmi/Config.in config BR2_PACKAGE_LIBQMI · line 24
Quoted 6 lines
package/libqmi/Config.in +6
-0+ 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?
Quoted 8 lines
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
Quoted 6 lines
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!
Signature · 2 lines
Aleksander https://aleksander.es
PATCH 1/1 libqmi: udev and qmi-over-mbim are optional
Hello,
Quoted 7 lines · Aleksander Morgado
Quoted 3 lines
... 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.
Quoted 1 line
Yes, probably that's a much simpler option. Will suggest a new patch.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 1/1 libqmi: udev and qmi-over-mbim are optional
In reply to Thomas PetazzoniQuoted 5 lines
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
Quoted 4 lines
else
... disable support
endif
Signature · 2 lines
Aleksander https://aleksander.es
PATCH 1/1 libqmi: udev and qmi-over-mbim are optional
Hello,
Quoted 8 lines · Aleksander Morgado
Quoted 4 lines
All in all, isn't it simpler to get rid of those options, and simply do:
ifeq ($(BR2_PACKAGE_LIBGUDEV),y)
... enable supportWhen 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 += fooBAR_CONF_OPTS += --enable-foo
elseBAR_CONF_OPTS += --disable-foo
endif
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com