* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
@ 2010-04-16 15:28 ` Dan Nicholson
2010-04-16 16:31 ` Kay Sievers
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Dan Nicholson @ 2010-04-16 15:28 UTC (permalink / raw)
To: linux-hotplug
On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
> Hi!
>
> Review the attached patches, please.
> From 5cc42c6f87f690fdf66e29de2a816dab49a119f4 Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 00:21:02 +0400
> Subject: [PATCH 1/2] Include linux/types.h
1. Prefix the subject with "hid2hci: " so it's clear from the shortlog what
this affects.
2. Please include some rational in the commit message. What prototype or
datatype is missing that requires including <linux/types.h>?
>
> ---
> extras/hid2hci/hid2hci.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/extras/hid2hci/hid2hci.c b/extras/hid2hci/hid2hci.c
> index 0d0a022..839c4fb 100644
> --- a/extras/hid2hci/hid2hci.c
> +++ b/extras/hid2hci/hid2hci.c
> @@ -28,6 +28,7 @@
> #include <string.h>
> #include <getopt.h>
> #include <sys/ioctl.h>
> +#include <linux/types.h>
> #include <linux/hiddev.h>
> #include <usb.h>
>
> --
> 1.7.0.5
>
>
> From f9881e554deafad1301d00e4539a02fe63078c00 Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 09:38:32 +0400
> Subject: [PATCH 2/2] Add --with-firmware-path configure option
Again, please include some rational in the commit message. What problem is
this solving?
>
> ---
> Makefile.am | 3 ++-
> configure.ac | 22 ++++++++++++++++++++++
> extras/firmware/firmware.c | 10 ++++++----
> 3 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 68a68d9..f094746 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -156,7 +156,7 @@ EXTRA_DIST += \
> udev/udevd.xml
>
> %.7 %.8 : %.xml
> - $(XSLTPROC) -o $@ -nonet http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
> + $(XSLTPROC) -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $<
This is an unrelated change. Please remove it from this patch.
>
> # ------------------------------------------------------------------------------
> # udev tests
> @@ -194,6 +194,7 @@ dist_udevrules_DATA += \
> # ------------------------------------------------------------------------------
> extras_firmware_firmware_SOURCES = extras/firmware/firmware.c
> extras_firmware_firmware_LDADD = libudev/libudev-private.la
> +extras_firmware_firmware_CPPFLAGS = $(AM_CPPFLAGS) -DFIRMWARE_PATH="$(FIRMWARE_PATH)"
> dist_udevrules_DATA += extras/firmware/50-firmware.rules
> libexec_PROGRAMS = extras/firmware/firmware
>
> diff --git a/configure.ac b/configure.ac
> index 492fa02..5defd5c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -82,6 +82,27 @@ if test "x$enable_extras" = xyes; then
> fi])
> AC_SUBST(PCI_DATABASE)
>
> + AC_ARG_WITH(firmware-path,
> + AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
> + [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
> + [],
> + [with_firmware_path="/lib/fimware/updates:/lib/fimware"]
> + )
> + [
> + OLD_IFS=$IFS
> + IFS=:
> + for i in $with_firmware_path
> + do
> + if [ "x${FIRMWARE_PATH}" = "x" ]; then
> + FIRMWARE_PATH="\\\"${i}/\\\""
> + else
> + FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
> + fi
> + done
> + IFS=$OLD_IFS
> + ]
I'm not sure what wrapping this whole section in [] quoting does. It looks
like a good handling of the option, though.
> + AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
> +
> AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
> AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
> fi
> @@ -144,6 +165,7 @@ AC_MSG_RESULT([
>
> usb.ids: ${USB_DATABASE}
> pci.ids: ${PCI_DATABASE}
> + firmware path: ${FIRMWARE_PATH}
>
> xsltproc: ${XSLTPROC}
> gperf: ${GPERF}
> diff --git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
> index 92f0918..76071db 100644
> --- a/extras/firmware/firmware.c
> +++ b/extras/firmware/firmware.c
> @@ -79,10 +79,7 @@ int main(int argc, char **argv)
> { "help", no_argument, NULL, 'h' },
> {}
> };
> - static const char *searchpath[] = {
> - "/lib/firmware/updates/",
> - "/lib/firmware/"
> - };
> + static const char *searchpath[] = { FIRMWARE_PATH };
> char fwencpath[UTIL_PATH_SIZE];
> char misspath[UTIL_PATH_SIZE];
> char loadpath[UTIL_PATH_SIZE];
> @@ -97,6 +94,11 @@ int main(int argc, char **argv)
> unsigned int i;
> int rc = 0;
>
> + for (i = 0; i < ARRAY_SIZE(searchpath); i++) {
> + printf("Path %s\n", searchpath[i]);
> + }
> + return 0;
> +
I'm pretty sure the program stops doing anything useful with this hunk. :)
> udev_log_init("firmware");
>
> for (;;) {
> --
> 1.7.0.5
Besides those small issues, both patches seem pretty good to me.
--
Dan
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
2010-04-16 15:28 ` Dan Nicholson
@ 2010-04-16 16:31 ` Kay Sievers
2010-04-16 17:34 ` Dan Nicholson
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Kay Sievers @ 2010-04-16 16:31 UTC (permalink / raw)
To: linux-hotplug
On Fri, Apr 16, 2010 at 17:28, Dan Nicholson <dbn.lists@gmail.com> wrote:
> On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -82,6 +82,27 @@ if test "x$enable_extras" = xyes; then
>> fi])
>> AC_SUBST(PCI_DATABASE)
>>
>> + AC_ARG_WITH(firmware-path,
>> + AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
>> + [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
Not sure if we need a list of directories here. Wouldn't the prefix
/lib/firmware be sufficient, the same way we handle all the other
configurable directories?
Kay
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
2010-04-16 15:28 ` Dan Nicholson
2010-04-16 16:31 ` Kay Sievers
@ 2010-04-16 17:34 ` Dan Nicholson
2010-04-16 17:51 ` Yury G. Kudryashov
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Dan Nicholson @ 2010-04-16 17:34 UTC (permalink / raw)
To: linux-hotplug
On Fri, Apr 16, 2010 at 9:31 AM, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Fri, Apr 16, 2010 at 17:28, Dan Nicholson <dbn.lists@gmail.com> wrote:
>> On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -82,6 +82,27 @@ if test "x$enable_extras" = xyes; then
>>> fi])
>>> AC_SUBST(PCI_DATABASE)
>>>
>>> + AC_ARG_WITH(firmware-path,
>>> + AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
>>> + [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
>
> Not sure if we need a list of directories here. Wouldn't the prefix
> /lib/firmware be sufficient, the same way we handle all the other
> configurable directories?
This is just maintaining the current default in firmware.c. The code
already deals with an array of directories, so you might as well let
people build in any crazy path they want.
--
Dan
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
` (2 preceding siblings ...)
2010-04-16 17:34 ` Dan Nicholson
@ 2010-04-16 17:51 ` Yury G. Kudryashov
2010-04-16 18:03 ` Dan Nicholson
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Yury G. Kudryashov @ 2010-04-16 17:51 UTC (permalink / raw)
To: linux-hotplug
Dan Nicholson wrote:
>> @@ -156,7 +156,7 @@ EXTRA_DIST += \
>> udev/udevd.xml
>>
>> %.7 %.8 : %.xml
>> - $(XSLTPROC) -o $@ -nonet
>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>> $<
>> + $(XSLTPROC) -o $@
>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>> $<
>
> This is an unrelated change. Please remove it from this patch.
Oops. Sorry. I should've reread the patch before sending.
>> + [
>> + OLD_IFS=$IFS
>> + IFS=:
>> + for i in $with_firmware_path
>> + do
>> + if [ "x${FIRMWARE_PATH}" = "x" ]; then
>> + FIRMWARE_PATH="\\\"${i}/\\\""
>> + else
>> + FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
>> + fi
>> + done
>> + IFS=$OLD_IFS
>> + ]
>
> I'm not sure what wrapping this whole section in [] quoting does. It looks
> like a good handling of the option, though.
It escapes "if [ ... ]".
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
` (3 preceding siblings ...)
2010-04-16 17:51 ` Yury G. Kudryashov
@ 2010-04-16 18:03 ` Dan Nicholson
2010-04-16 18:52 ` Yury G. Kudryashov
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Dan Nicholson @ 2010-04-16 18:03 UTC (permalink / raw)
To: linux-hotplug
On Fri, Apr 16, 2010 at 10:51 AM, Yury G. Kudryashov <urkud@ya.ru> wrote:
> Dan Nicholson wrote:
>
>>> @@ -156,7 +156,7 @@ EXTRA_DIST += \
>>> udev/udevd.xml
>>>
>>> %.7 %.8 : %.xml
>>> - $(XSLTPROC) -o $@ -nonet
>>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>>> $<
>>> + $(XSLTPROC) -o $@
>>> http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl
>>> $<
>>
>> This is an unrelated change. Please remove it from this patch.
> Oops. Sorry. I should've reread the patch before sending.
>>> + [
>>> + OLD_IFS=$IFS
>>> + IFS=:
>>> + for i in $with_firmware_path
>>> + do
>>> + if [ "x${FIRMWARE_PATH}" = "x" ]; then
>>> + FIRMWARE_PATH="\\\"${i}/\\\""
>>> + else
>>> + FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
>>> + fi
>>> + done
>>> + IFS=$OLD_IFS
>>> + ]
>>
>> I'm not sure what wrapping this whole section in [] quoting does. It looks
>> like a good handling of the option, though.
> It escapes "if [ ... ]".
Ah. You can also use 'test "x$FIRMWARE_PATH" = x' to avoid that. Also,
I should mention that you should use '=' instead of '=' since the
former is portable across shells.
--
Dan
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
` (4 preceding siblings ...)
2010-04-16 18:03 ` Dan Nicholson
@ 2010-04-16 18:52 ` Yury G. Kudryashov
2010-04-16 18:56 ` Yury G. Kudryashov
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Yury G. Kudryashov @ 2010-04-16 18:52 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]
Dan Nicholson wrote:
> On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
>> Hi!
>>
>> Review the attached patches, please.
>
>> From 5cc42c6f87f690fdf66e29de2a816dab49a119f4 Mon Sep 17 00:00:00 2001
>> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
>> Date: Fri, 16 Apr 2010 00:21:02 +0400
>> Subject: [PATCH 1/2] Include linux/types.h
>
> 1. Prefix the subject with "hid2hci: " so it's clear from the shortlog
> what
> this affects.
>
> 2. Please include some rational in the commit message. What prototype or
> datatype is missing that requires including <linux/types.h>?
Done.
>> From f9881e554deafad1301d00e4539a02fe63078c00 Mon Sep 17 00:00:00 2001
>> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
>> Date: Fri, 16 Apr 2010 09:38:32 +0400
>> Subject: [PATCH 2/2] Add --with-firmware-path configure option
>
> Again, please include some rational in the commit message. What problem is
> this solving?
Done.
>> @@ -97,6 +94,11 @@ int main(int argc, char **argv)
>> unsigned int i;
>> int rc = 0;
>>
>> + for (i = 0; i < ARRAY_SIZE(searchpath); i++) {
>> + printf("Path %s\n", searchpath[i]);
>> + }
>> + return 0;
>> +
>
> I'm pretty sure the program stops doing anything useful with this hunk. :)
I really should stop posting patches after midnight. I hope that the
attached version is better.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-hid2hci-include-linux-types.h-for-__u32.patch --]
[-- Type: text/x-patch; name="0001-hid2hci-include-linux-types.h-for-__u32.patch", Size: 798 bytes --]
From 6d2d2d4d2c665539c140bbf471c8d1e30baf508b Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 00:21:02 +0400
Subject: [PATCH 1/2] hid2hci: include linux/types.h for __u32
__u32 in linux/hiddev.h is undefined on my laptop (linux-headers-2.6.28.5).
Probably, this is a bug in kernel headers or in my distro but this would not hurt.
---
| 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
--git a/extras/hid2hci/hid2hci.c b/extras/hid2hci/hid2hci.c
index 0d0a022..839c4fb 100644
--- a/extras/hid2hci/hid2hci.c
+++ b/extras/hid2hci/hid2hci.c
@@ -28,6 +28,7 @@
#include <string.h>
#include <getopt.h>
#include <sys/ioctl.h>
+#include <linux/types.h>
#include <linux/hiddev.h>
#include <usb.h>
--
1.7.0.5
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-with-firmware-path-configure-option.patch --]
[-- Type: text/x-patch; name="0002-Add-with-firmware-path-configure-option.patch", Size: 2105 bytes --]
From c810304553d585bc71ffd5633c3cd9241fd2ce15 Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 09:38:32 +0400
Subject: [PATCH 2/2] Add --with-firmware-path configure option
In NixOS we need to use non-standard firmware path: we have no /lib.
---
configure.ac | 21 +++++++++++++++++++++
| 5 +----
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index 492fa02..cce70d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,26 @@ if test "x$enable_extras" = xyes; then
fi])
AC_SUBST(PCI_DATABASE)
+ AC_ARG_WITH(firmware-path,
+ AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
+ [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
+ [],
+ [with_firmware_path="/lib/fimware/updates:/lib/fimware"]
+ )
+ OLD_IFS=$IFS
+ IFS=:
+ for i in $with_firmware_path
+ do
+ if test "x${FIRMWARE_PATH}" = "x"
+ then
+ FIRMWARE_PATH="\\\"${i}/\\\""
+ else
+ FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
+ fi
+ done
+ IFS=$OLD_IFS
+ AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
+
AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
fi
@@ -144,6 +164,7 @@ AC_MSG_RESULT([
usb.ids: ${USB_DATABASE}
pci.ids: ${PCI_DATABASE}
+ firmware path: ${FIRMWARE_PATH}
xsltproc: ${XSLTPROC}
gperf: ${GPERF}
--git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
index 92f0918..76593ba 100644
--- a/extras/firmware/firmware.c
+++ b/extras/firmware/firmware.c
@@ -79,10 +79,7 @@ int main(int argc, char **argv)
{ "help", no_argument, NULL, 'h' },
{}
};
- static const char *searchpath[] = {
- "/lib/firmware/updates/",
- "/lib/firmware/"
- };
+ static const char *searchpath[] = { FIRMWARE_PATH };
char fwencpath[UTIL_PATH_SIZE];
char misspath[UTIL_PATH_SIZE];
char loadpath[UTIL_PATH_SIZE];
--
1.7.0.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
` (5 preceding siblings ...)
2010-04-16 18:52 ` Yury G. Kudryashov
@ 2010-04-16 18:56 ` Yury G. Kudryashov
2010-04-16 19:01 ` Yury G. Kudryashov
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Yury G. Kudryashov @ 2010-04-16 18:56 UTC (permalink / raw)
To: linux-hotplug
Yury G. Kudryashov wrote:
Ignore the previous message. It doesn't compile.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
` (6 preceding siblings ...)
2010-04-16 18:56 ` Yury G. Kudryashov
@ 2010-04-16 19:01 ` Yury G. Kudryashov
2010-04-16 22:01 ` Dan Nicholson
2010-04-16 22:02 ` Dan Nicholson
9 siblings, 0 replies; 11+ messages in thread
From: Yury G. Kudryashov @ 2010-04-16 19:01 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 270 bytes --]
Yury G. Kudryashov wrote:
> I really should stop posting patches after midnight. I hope that the
> attached version is better.
I should also stop posting "fixed" versions of patches without re-compiling.
First patch is OK, new version of the second patch is attached.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Add-with-firmware-path-configure-option.patch --]
[-- Type: text/x-patch; name="0002-Add-with-firmware-path-configure-option.patch", Size: 2690 bytes --]
From 499a8e76e856b1cec3f045407eeb52a5c3d85a18 Mon Sep 17 00:00:00 2001
From: Yury G. Kudryashov <urkud.urkud@gmail.com>
Date: Fri, 16 Apr 2010 09:38:32 +0400
Subject: [PATCH 2/2] Add --with-firmware-path configure option
In NixOS we need to use non-standard firmware path: we have no /lib.
---
Makefile.am | 1 +
configure.ac | 21 +++++++++++++++++++++
| 5 +----
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 68a68d9..3b628a4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -194,6 +194,7 @@ dist_udevrules_DATA += \
# ------------------------------------------------------------------------------
extras_firmware_firmware_SOURCES = extras/firmware/firmware.c
extras_firmware_firmware_LDADD = libudev/libudev-private.la
+extras_firmware_firmware_CPPFLAGS = $(AM_CPPFLAGS) -DFIRMWARE_PATH="$(FIRMWARE_PATH)"
dist_udevrules_DATA += extras/firmware/50-firmware.rules
libexec_PROGRAMS = extras/firmware/firmware
diff --git a/configure.ac b/configure.ac
index 492fa02..cce70d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,26 @@ if test "x$enable_extras" = xyes; then
fi])
AC_SUBST(PCI_DATABASE)
+ AC_ARG_WITH(firmware-path,
+ AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
+ [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
+ [],
+ [with_firmware_path="/lib/fimware/updates:/lib/fimware"]
+ )
+ OLD_IFS=$IFS
+ IFS=:
+ for i in $with_firmware_path
+ do
+ if test "x${FIRMWARE_PATH}" = "x"
+ then
+ FIRMWARE_PATH="\\\"${i}/\\\""
+ else
+ FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
+ fi
+ done
+ IFS=$OLD_IFS
+ AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
+
AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
fi
@@ -144,6 +164,7 @@ AC_MSG_RESULT([
usb.ids: ${USB_DATABASE}
pci.ids: ${PCI_DATABASE}
+ firmware path: ${FIRMWARE_PATH}
xsltproc: ${XSLTPROC}
gperf: ${GPERF}
--git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
index 92f0918..76593ba 100644
--- a/extras/firmware/firmware.c
+++ b/extras/firmware/firmware.c
@@ -79,10 +79,7 @@ int main(int argc, char **argv)
{ "help", no_argument, NULL, 'h' },
{}
};
- static const char *searchpath[] = {
- "/lib/firmware/updates/",
- "/lib/firmware/"
- };
+ static const char *searchpath[] = { FIRMWARE_PATH };
char fwencpath[UTIL_PATH_SIZE];
char misspath[UTIL_PATH_SIZE];
char loadpath[UTIL_PATH_SIZE];
--
1.7.0.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
` (7 preceding siblings ...)
2010-04-16 19:01 ` Yury G. Kudryashov
@ 2010-04-16 22:01 ` Dan Nicholson
2010-04-16 22:02 ` Dan Nicholson
9 siblings, 0 replies; 11+ messages in thread
From: Dan Nicholson @ 2010-04-16 22:01 UTC (permalink / raw)
To: linux-hotplug
On Fri, Apr 16, 2010 at 10:52:44PM +0400, Yury G. Kudryashov wrote:
> Dan Nicholson wrote:
>
> > On Fri, Apr 16, 2010 at 09:50:42AM +0400, Yury G. Kudryashov wrote:
> >> Hi!
> >>
> >> Review the attached patches, please.
> >
> >> From 5cc42c6f87f690fdf66e29de2a816dab49a119f4 Mon Sep 17 00:00:00 2001
> >> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> >> Date: Fri, 16 Apr 2010 00:21:02 +0400
> >> Subject: [PATCH 1/2] Include linux/types.h
> >
> > 1. Prefix the subject with "hid2hci: " so it's clear from the shortlog
> > what
> > this affects.
> >
> > 2. Please include some rational in the commit message. What prototype or
> > datatype is missing that requires including <linux/types.h>?
> Done.
> >> From f9881e554deafad1301d00e4539a02fe63078c00 Mon Sep 17 00:00:00 2001
> >> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> >> Date: Fri, 16 Apr 2010 09:38:32 +0400
> >> Subject: [PATCH 2/2] Add --with-firmware-path configure option
> >
> > Again, please include some rational in the commit message. What problem is
> > this solving?
> Done.
> >> @@ -97,6 +94,11 @@ int main(int argc, char **argv)
> >> unsigned int i;
> >> int rc = 0;
> >>
> >> + for (i = 0; i < ARRAY_SIZE(searchpath); i++) {
> >> + printf("Path %s\n", searchpath[i]);
> >> + }
> >> + return 0;
> >> +
> >
> > I'm pretty sure the program stops doing anything useful with this hunk. :)
> I really should stop posting patches after midnight. I hope that the
> attached version is better.
> From 6d2d2d4d2c665539c140bbf471c8d1e30baf508b Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 00:21:02 +0400
> Subject: [PATCH 1/2] hid2hci: include linux/types.h for __u32
>
> __u32 in linux/hiddev.h is undefined on my laptop (linux-headers-2.6.28.5).
> Probably, this is a bug in kernel headers or in my distro but this would not hurt.
Looks good. I see you resent the other patch.
--
Dan
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: two patches
2010-04-16 5:50 two patches Yury G. Kudryashov
` (8 preceding siblings ...)
2010-04-16 22:01 ` Dan Nicholson
@ 2010-04-16 22:02 ` Dan Nicholson
9 siblings, 0 replies; 11+ messages in thread
From: Dan Nicholson @ 2010-04-16 22:02 UTC (permalink / raw)
To: linux-hotplug
On Fri, Apr 16, 2010 at 11:01:31PM +0400, Yury G. Kudryashov wrote:
> Yury G. Kudryashov wrote:
>
> > I really should stop posting patches after midnight. I hope that the
> > attached version is better.
> I should also stop posting "fixed" versions of patches without re-compiling.
> First patch is OK, new version of the second patch is attached.
> From 499a8e76e856b1cec3f045407eeb52a5c3d85a18 Mon Sep 17 00:00:00 2001
> From: Yury G. Kudryashov <urkud.urkud@gmail.com>
> Date: Fri, 16 Apr 2010 09:38:32 +0400
> Subject: [PATCH 2/2] Add --with-firmware-path configure option
>
> In NixOS we need to use non-standard firmware path: we have no /lib.
> ---
> Makefile.am | 1 +
> configure.ac | 21 +++++++++++++++++++++
> extras/firmware/firmware.c | 5 +----
> 3 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 68a68d9..3b628a4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -194,6 +194,7 @@ dist_udevrules_DATA += \
> # ------------------------------------------------------------------------------
> extras_firmware_firmware_SOURCES = extras/firmware/firmware.c
> extras_firmware_firmware_LDADD = libudev/libudev-private.la
> +extras_firmware_firmware_CPPFLAGS = $(AM_CPPFLAGS) -DFIRMWARE_PATH="$(FIRMWARE_PATH)"
> dist_udevrules_DATA += extras/firmware/50-firmware.rules
> libexec_PROGRAMS = extras/firmware/firmware
>
> diff --git a/configure.ac b/configure.ac
> index 492fa02..cce70d1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -82,6 +82,26 @@ if test "x$enable_extras" = xyes; then
> fi])
> AC_SUBST(PCI_DATABASE)
>
> + AC_ARG_WITH(firmware-path,
> + AS_HELP_STRING([--with-firmware-path=DIR[[[:DIR[...]]]]],
> + [Firmware search path (default=/lib/firmware/updates:/lib/firmware)]),
> + [],
> + [with_firmware_path="/lib/fimware/updates:/lib/fimware"]
> + )
> + OLD_IFS=$IFS
> + IFS=:
> + for i in $with_firmware_path
> + do
> + if test "x${FIRMWARE_PATH}" = "x"
> + then
> + FIRMWARE_PATH="\\\"${i}/\\\""
> + else
> + FIRMWARE_PATH="${FIRMWARE_PATH}, \\\"${i}/\\\""
> + fi
> + done
> + IFS=$OLD_IFS
> + AC_SUBST([FIRMWARE_PATH], [$FIRMWARE_PATH])
> +
> AC_CHECK_HEADER([linux/input.h], [:], AC_MSG_ERROR([kernel headers not found]))
> AC_SUBST([INCLUDE_PREFIX], [$(echo '#include <linux/input.h>' | eval $ac_cpp -E - | sed -n '/linux\/input.h/ {s:.*"\(.*\)/linux/input.h".*:\1:; p; q}')])
> fi
> @@ -144,6 +164,7 @@ AC_MSG_RESULT([
>
> usb.ids: ${USB_DATABASE}
> pci.ids: ${PCI_DATABASE}
> + firmware path: ${FIRMWARE_PATH}
>
> xsltproc: ${XSLTPROC}
> gperf: ${GPERF}
> diff --git a/extras/firmware/firmware.c b/extras/firmware/firmware.c
> index 92f0918..76593ba 100644
> --- a/extras/firmware/firmware.c
> +++ b/extras/firmware/firmware.c
> @@ -79,10 +79,7 @@ int main(int argc, char **argv)
> { "help", no_argument, NULL, 'h' },
> {}
> };
> - static const char *searchpath[] = {
> - "/lib/firmware/updates/",
> - "/lib/firmware/"
> - };
> + static const char *searchpath[] = { FIRMWARE_PATH };
> char fwencpath[UTIL_PATH_SIZE];
> char misspath[UTIL_PATH_SIZE];
> char loadpath[UTIL_PATH_SIZE];
> --
> 1.7.0.5
Looks good on inspection, although I didn't test it.
--
Dan
^ permalink raw reply [flat|nested] 11+ messages in thread