Op 07-08-13 02:26, Andy Lutomirski schreef:
On Tue, Aug 6, 2013 at 5:24 PM, Tom Gundersen t...@jklm.no wrote:
On 6 Aug 2013 18:32, Bryan Kadzban br...@kadzban.is-a-geek.net wrote:
On Tue, Aug 06, 2013 at 11:17:17AM +0200, Tom Gundersen wrote:
On Tue, Aug 6, 2013 at 11:11 AM, Tom Gundersen t...@jklm.no wrote:
On Tue, Aug 6, 2013 at 10:20 AM, Maarten Lankhorst
m.b.lankho...@gmail.com wrote:
Op 05-08-13 18:29, Andy Lutomirski schreef:
The systemd commit below can delay firmware loading by multiple
minutes if CONFIG_FW_LOADER_USER_HELPER=y. Unfortunately no one
noticed that the systemd-udev change would break new kernels as well
as old kernels.
Since the kernel apparently can't count on reasonable userspace
support, turn this thing off by default.
commit a3bd8447be4ea2ce230eb8ae0e815c04d85fa15a
Author: Tom Gundersen t...@jklm.no
Date: Mon Mar 18 15:12:18 2013 +0100
udev: make firmware loading optional and disable by default
Distros that whish to support old kernels should set
--with-firmware-dirs=/usr/lib/firmware/updates:/usr/lib/firmware
to retain the old behaviour.
methinks this patch should be reverted then,
Well, all the code is still there, so it can be enabled if anyone
wants it.
or a stub should be added to udev to always fail firmware loading so
timeouts don't occur.
I think the only use (if any) of a userspace firmware loader would be
for anyone who wants a custom one (i.e., not udev), so we shouldn't
just fail the loading from udev unconditionally.
How about we just improve the udev documentation a bit, similar to
Andy's kernel patch?
Sorry, I should first have checked. We already document this in the
README:
Userspace firmware loading is deprecated, will go away, and
sometimes causes problems:
CONFIG_FW_LOADER_USER_HELPER=n
...And this patch is making the kernel default to the correct behavior,
instead of the now-broken-by-udev behavior.
I'm not sure I see the issue with it? :-)
Oh yeah this patch is totally the right thing to do, I was just arguing that
there is nothing to be done on the udev side.
(Add me to the list of people that think udev is broken too, fwiw. But
let's at least not leave *both* sides in a broken-by-default state.)
Well I don't think it is too much to ask that the kernel and udev should be
configured in a consistent way. Especially as thing still work even if you
get it wrong, albeit with a delay.
Except that the current defaults are inconsistent and there is no
explanation anywhere in the logs when this is screwed up.
So what is wrong with my 'fail in udev immediately if not configured' idea? In
that case it
doesn't matter whether CONFIG_FW_LOADER_USER_HELPER is set or not.
You could even print a useful message for the user in udev to the log, so they
have an idea of what
happened. Breaking udev on older still supported kernels by default without
printing any debug info
is silly, and the only cost is a small increase in disk space when unused. I
did so in below patch.
~Maarten
I converted ELEMENTSOF to != ELEMENTSOF in the loop to work correctly when
the array is empty.
Most code in udev-builtin-firmware is eliminated at -O2 optimization level when
FIRMWARE_PATH
is not explicitly set.
8
diff --git a/Makefile.am b/Makefile.am
index b8b8d06..2097629 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2235,6 +2235,7 @@ libudev_core_la_SOURCES = \
src/udev/udev-ctrl.c \
src/udev/udev-builtin.c \
src/udev/udev-builtin-btrfs.c \
+ src/udev/udev-builtin-firmware.c \
src/udev/udev-builtin-hwdb.c \
src/udev/udev-builtin-input_id.c \
src/udev/udev-builtin-keyboard.c \
@@ -2271,14 +2272,6 @@ libudev_core_la_CPPFLAGS = \
$(AM_CPPFLAGS) \
-DFIRMWARE_PATH=$(FIRMWARE_PATH)
-if ENABLE_FIRMWARE
-libudev_core_la_SOURCES += \
- src/udev/udev-builtin-firmware.c
-
-dist_udevrules_DATA += \
- rules/50-firmware.rules
-endif
-
if HAVE_KMOD
libudev_core_la_SOURCES += \
src/udev/udev-builtin-kmod.c
diff --git a/configure.ac b/configure.ac
index 0ecc716..dc7a3e3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -823,8 +823,6 @@ for i in $with_firmware_path; do
done
IFS=$OLD_IFS
AC_SUBST(FIRMWARE_PATH)
-AS_IF([test x${FIRMWARE_PATH} != x], [ AC_DEFINE(HAVE_FIRMWARE, 1, [Define
if FIRMWARE is available]) ])
-AM_CONDITIONAL(ENABLE_FIRMWARE, [test x${FIRMWARE_PATH} != x])
#
--
AC_ARG_ENABLE([gudev],
diff --git a/rules/50-firmware.rules b/rules/50-firmware.rules
deleted file mode 100644
index f0ae684..000
--- a/rules/50-firmware.rules
+++ /dev/null
@@ -1,3 +0,0 @@
-# do not edit this file, it will be overwritten on update
-
-SUBSYSTEM==firmware, ACTION==add, RUN{builtin}=firmware
diff --git a/rules/50-udev-default.rules b/rules/50-udev-default.rules
index f764789..645830e 100644
--- a/rules/50-udev-default.rules
+++ b
