* [Buildroot] [PATCH v3] package/iqvlinux: new package
@ 2015-10-09 13:26 Romain Naour
2015-10-10 15:04 ` Yann E. MORIN
2015-10-12 21:17 ` Thomas Petazzoni
0 siblings, 2 replies; 11+ messages in thread
From: Romain Naour @ 2015-10-09 13:26 UTC (permalink / raw)
To: buildroot
The PCI support needs to be checked since this driver is based
on it. Otherwise the build fail with:
#error "This driver requires PCI support to be available"
But this message is concealed by several occurrence of this
one:
error: implicit declaration of function 'pci_find_bus' [-Werror=implicit-function-declaration]
Signed-off-by: Romain Naour <romain.naour@openwide.fr>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
v3: changes suggested by Arnout
- remove the patch and use CC=$(TARGET_CC) instead.
- Add a 'or' between GPLv2 and BSD-3c for license information
- Add some more license files (files.txt)
- Fixes typos and indentation.
- move the check for PCI option to LINUX_POST_CONFIGURE_HOOKS
- redirect echo to stderr.
v2: - rename the package simply to iqvlinux (ThomasP)
- move it to "Hardware Handling" menu (ThomasP)
- Cc Yann for the kernel-module infra
- Add a check for CONFIG_PCI even if it's redundant with
the message from the Makefile.
(Do we really need this check ?)
---
package/Config.in | 1 +
package/iqvlinux/Config.in | 18 ++++++++++++++++++
package/iqvlinux/iqvlinux.hash | 5 +++++
package/iqvlinux/iqvlinux.mk | 34 ++++++++++++++++++++++++++++++++++
4 files changed, 58 insertions(+)
create mode 100644 package/iqvlinux/Config.in
create mode 100644 package/iqvlinux/iqvlinux.hash
create mode 100644 package/iqvlinux/iqvlinux.mk
diff --git a/package/Config.in b/package/Config.in
index 3794f44..5e2ac80 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -364,6 +364,7 @@ endif
source "package/iostat/Config.in"
source "package/ipmitool/Config.in"
source "package/ipmiutil/Config.in"
+ source "package/iqvlinux/Config.in"
source "package/irda-utils/Config.in"
source "package/iucode-tool/Config.in"
source "package/kbd/Config.in"
diff --git a/package/iqvlinux/Config.in b/package/iqvlinux/Config.in
new file mode 100644
index 0000000..275c67e
--- /dev/null
+++ b/package/iqvlinux/Config.in
@@ -0,0 +1,18 @@
+config BR2_PACKAGE_IQVLINUX
+ bool "iqvlinux"
+ depends on BR2_LINUX_KERNEL
+ help
+ Intel Ethernet Adapter Debug Driver for Linux (iqvlinux),
+ which supports kernel versions 2.6.x up through 4.0.x.
+
+ This debug driver supports all Intel's networking Tools based
+ on the SDK version 2.19.36.0 or higher which support Intel
+ Enthernet chip including e1000, e1000e, i210...
+
+ Note: This driver requires PCI support to be enabled
+ (i.e. CONFIG_PCI).
+
+ http://sourceforge.net/projects/e1000/files/iqvlinux/
+
+comment "iqvlinux needs a Linux kernel to be built"
+ depends on !BR2_LINUX_KERNEL
diff --git a/package/iqvlinux/iqvlinux.hash b/package/iqvlinux/iqvlinux.hash
new file mode 100644
index 0000000..ddf57b7
--- /dev/null
+++ b/package/iqvlinux/iqvlinux.hash
@@ -0,0 +1,5 @@
+# From http://sourceforge.net/projects/e1000/files/iqvlinux/1.1.5.3/
+sha1 bd94416e4364015dbbd78a22e51080bf7ea81fac iqvlinux.tar.gz
+md5 fb6a2a4dc122d39070fcb06985c97a05 iqvlinux.tar.gz
+# locally computed
+sha256 8cb19f3bfe040100a13bb2d05cb2b54f2b259e55cef23f8cc5aa6f2f31e98bec iqvlinux.tar.gz
diff --git a/package/iqvlinux/iqvlinux.mk b/package/iqvlinux/iqvlinux.mk
new file mode 100644
index 0000000..ed98f1f
--- /dev/null
+++ b/package/iqvlinux/iqvlinux.mk
@@ -0,0 +1,34 @@
+################################################################################
+#
+# iqvlinux
+#
+################################################################################
+
+IQVLINUX_VERSION = 1.1.5.3
+
+IQVLINUX_SITE = \
+ http://sourceforge.net/projects/e1000/files/iqvlinux/$(IQVLINUX_VERSION)
+IQVLINUX_SOURCE = iqvlinux.tar.gz
+
+IQVLINUX_LICENSE = GPLv2 or BSD-3c
+IQVLINUX_LICENSE_FILES = COPYING src/linux/driver/files.txt \
+ inc/linux/files.txt inc/files.txt
+
+IQVLINUX_MODULE_MAKE_OPTS = NALDIR=$(@D) KSRC=$(LINUX_DIR) CC=$(TARGET_CC)
+
+IQVLINUX_MODULE_SUBDIRS = src/linux/driver
+
+ifeq ($(BR2_PACKAGE_IQVLINUX),y)
+define IQVLINUX_PCI_CHECK
+ @if ! grep -Fqx 'CONFIG_PCI=y' $(LINUX_DIR)/.config; then \
+ echo "ERROR: Enable CONFIG_PCI in the linux kernel config." 1>&2 ; \
+ exit 1; \
+ fi
+endef
+
+# Check if PCI is enabled in the Linux kernel build by Buildroot.
+LINUX_POST_CONFIGURE_HOOKS += IQVLINUX_PCI_CHECK
+endif
+
+$(eval $(kernel-module))
+$(eval $(generic-package))
--
2.4.3
^ permalink raw reply related [flat|nested] 11+ messages in thread* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-09 13:26 [Buildroot] [PATCH v3] package/iqvlinux: new package Romain Naour
@ 2015-10-10 15:04 ` Yann E. MORIN
2015-10-12 20:44 ` Thomas Petazzoni
2015-10-12 21:16 ` Thomas Petazzoni
2015-10-12 21:17 ` Thomas Petazzoni
1 sibling, 2 replies; 11+ messages in thread
From: Yann E. MORIN @ 2015-10-10 15:04 UTC (permalink / raw)
To: buildroot
Romain, All,
On 2015-10-09 15:26 +0200, Romain Naour spake thusly:
> The PCI support needs to be checked since this driver is based
> on it. Otherwise the build fail with:
> #error "This driver requires PCI support to be available"
>
> But this message is concealed by several occurrence of this
> one:
> error: implicit declaration of function 'pci_find_bus' [-Werror=implicit-function-declaration]
>
> Signed-off-by: Romain Naour <romain.naour@openwide.fr>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Arnout Vandecappelle <arnout@mind.be>
[--SNIP--]
> diff --git a/package/iqvlinux/Config.in b/package/iqvlinux/Config.in
> new file mode 100644
> index 0000000..275c67e
> --- /dev/null
> +++ b/package/iqvlinux/Config.in
> @@ -0,0 +1,18 @@
> +config BR2_PACKAGE_IQVLINUX
> + bool "iqvlinux"
> + depends on BR2_LINUX_KERNEL
> + help
> + Intel Ethernet Adapter Debug Driver for Linux (iqvlinux),
> + which supports kernel versions 2.6.x up through 4.0.x.
What about 4.1+ ?
Should we do a version-check at configure time, or is there such a check
already done by the package itself?
[--SNIP--]
> diff --git a/package/iqvlinux/iqvlinux.mk b/package/iqvlinux/iqvlinux.mk
> new file mode 100644
> index 0000000..ed98f1f
> --- /dev/null
> +++ b/package/iqvlinux/iqvlinux.mk
> @@ -0,0 +1,34 @@
> +################################################################################
> +#
> +# iqvlinux
> +#
> +################################################################################
> +
> +IQVLINUX_VERSION = 1.1.5.3
> +
> +IQVLINUX_SITE = \
> + http://sourceforge.net/projects/e1000/files/iqvlinux/$(IQVLINUX_VERSION)
> +IQVLINUX_SOURCE = iqvlinux.tar.gz
> +
> +IQVLINUX_LICENSE = GPLv2 or BSD-3c
> +IQVLINUX_LICENSE_FILES = COPYING src/linux/driver/files.txt \
> + inc/linux/files.txt inc/files.txt
> +
> +IQVLINUX_MODULE_MAKE_OPTS = NALDIR=$(@D) KSRC=$(LINUX_DIR) CC=$(TARGET_CC)
Are you sure CC is needed? The kernel buildsystem should already set it, no?
> +IQVLINUX_MODULE_SUBDIRS = src/linux/driver
> +
> +ifeq ($(BR2_PACKAGE_IQVLINUX),y)
> +define IQVLINUX_PCI_CHECK
> + @if ! grep -Fqx 'CONFIG_PCI=y' $(LINUX_DIR)/.config; then \
> + echo "ERROR: Enable CONFIG_PCI in the linux kernel config." 1>&2 ; \
> + exit 1; \
> + fi
> +endef
> +
> +# Check if PCI is enabled in the Linux kernel build by Buildroot.
> +LINUX_POST_CONFIGURE_HOOKS += IQVLINUX_PCI_CHECK
> +endif
I really do not like that a package meddles in the affairs of another
package, like setting hooks for it.
I'd rather we add a variable that packages could set to require specific
kernel config options, something like:
IQVLINUX_LINUX_NEEDS_CONFIG_OPTS = CONFIG_PCI
Then in package/pkg-generic, in the inner-generic-package macro:
LINUX_NEEDS_CONFIG_OPTS += $$($(2)_NEEDS_LINUX_CONFIG)
And finally in linux/linux.mk:
define LINUX_CHECK_CONFIG_OPTS
$(foreach cfg,$(strip $(LINUX_NEEDS_CONFIG_OPTS)),\
if ! grep -E '^$(cfg)=y$$' $(@D)/.config >/dev/null; then \
printf "ERROR: Enable %s in your linux kernel config." "$(cfg)"; \
exit 1; \
fi;)
endef
LINUX_POST_CONFIGURE_HOOKS += LINUX_CHECK_CONFIG_OPTS
Regards,
Yann E. MORIN.
> +$(eval $(kernel-module))
> +$(eval $(generic-package))
> --
> 2.4.3
>
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 11+ messages in thread* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-10 15:04 ` Yann E. MORIN
@ 2015-10-12 20:44 ` Thomas Petazzoni
2015-10-12 21:16 ` Yann E. MORIN
2015-10-12 21:16 ` Thomas Petazzoni
1 sibling, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2015-10-12 20:44 UTC (permalink / raw)
To: buildroot
Hello,
On Sat, 10 Oct 2015 17:04:57 +0200, Yann E. MORIN wrote:
> > diff --git a/package/iqvlinux/Config.in b/package/iqvlinux/Config.in
> > new file mode 100644
> > index 0000000..275c67e
> > --- /dev/null
> > +++ b/package/iqvlinux/Config.in
> > @@ -0,0 +1,18 @@
> > +config BR2_PACKAGE_IQVLINUX
> > + bool "iqvlinux"
> > + depends on BR2_LINUX_KERNEL
> > + help
> > + Intel Ethernet Adapter Debug Driver for Linux (iqvlinux),
> > + which supports kernel versions 2.6.x up through 4.0.x.
>
> What about 4.1+ ?
>
> Should we do a version-check at configure time, or is there such a check
> already done by the package itself?
I think we don't really care. There are lots of other kernel modules
or Linux kernel extensions (Xenomai, RTAI) for which we don't do such
version checks.
> > +IQVLINUX_MODULE_SUBDIRS = src/linux/driver
> > +
> > +ifeq ($(BR2_PACKAGE_IQVLINUX),y)
> > +define IQVLINUX_PCI_CHECK
> > + @if ! grep -Fqx 'CONFIG_PCI=y' $(LINUX_DIR)/.config; then \
> > + echo "ERROR: Enable CONFIG_PCI in the linux kernel config." 1>&2 ; \
> > + exit 1; \
> > + fi
> > +endef
> > +
> > +# Check if PCI is enabled in the Linux kernel build by Buildroot.
> > +LINUX_POST_CONFIGURE_HOOKS += IQVLINUX_PCI_CHECK
> > +endif
>
> I really do not like that a package meddles in the affairs of another
> package, like setting hooks for it.
>
> I'd rather we add a variable that packages could set to require specific
> kernel config options, something like:
>
> IQVLINUX_LINUX_NEEDS_CONFIG_OPTS = CONFIG_PCI
>
> Then in package/pkg-generic, in the inner-generic-package macro:
>
> LINUX_NEEDS_CONFIG_OPTS += $$($(2)_NEEDS_LINUX_CONFIG)
>
> And finally in linux/linux.mk:
>
> define LINUX_CHECK_CONFIG_OPTS
> $(foreach cfg,$(strip $(LINUX_NEEDS_CONFIG_OPTS)),\
> if ! grep -E '^$(cfg)=y$$' $(@D)/.config >/dev/null; then \
> printf "ERROR: Enable %s in your linux kernel config." "$(cfg)"; \
> exit 1; \
> fi;)
> endef
> LINUX_POST_CONFIGURE_HOOKS += LINUX_CHECK_CONFIG_OPTS
I agree on the principle, but I think we should rather handle that like
we do for other options: enable automatically the needed option.
Remember how someone proposed to check for CONFIG_MODULES=y and error
out if not available? Instead, you implemented
b7c32c98bc810b88e0391117f225658f82b44995.
So a little bit in the direction of what you proposed, what about
refactoring the LINUX_NEEDS_MODULES mechanism into a more generic
mechanism that allows package to express which kernel option(s) they
need, and automatically enable such options in linux/linux.mk when
configuring the kernel.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-12 20:44 ` Thomas Petazzoni
@ 2015-10-12 21:16 ` Yann E. MORIN
2015-10-12 21:24 ` Thomas Petazzoni
0 siblings, 1 reply; 11+ messages in thread
From: Yann E. MORIN @ 2015-10-12 21:16 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2015-10-12 22:44 +0200, Thomas Petazzoni spake thusly:
> On Sat, 10 Oct 2015 17:04:57 +0200, Yann E. MORIN wrote:
[--SNIP--]
> > > +ifeq ($(BR2_PACKAGE_IQVLINUX),y)
> > > +define IQVLINUX_PCI_CHECK
> > > + @if ! grep -Fqx 'CONFIG_PCI=y' $(LINUX_DIR)/.config; then \
> > > + echo "ERROR: Enable CONFIG_PCI in the linux kernel config." 1>&2 ; \
> > > + exit 1; \
> > > + fi
> > > +endef
> > > +
> > > +# Check if PCI is enabled in the Linux kernel build by Buildroot.
> > > +LINUX_POST_CONFIGURE_HOOKS += IQVLINUX_PCI_CHECK
> > > +endif
> >
> > I really do not like that a package meddles in the affairs of another
> > package, like setting hooks for it.
> >
> > I'd rather we add a variable that packages could set to require specific
> > kernel config options, something like:
> >
> > IQVLINUX_LINUX_NEEDS_CONFIG_OPTS = CONFIG_PCI
> >
> > Then in package/pkg-generic, in the inner-generic-package macro:
> >
> > LINUX_NEEDS_CONFIG_OPTS += $$($(2)_NEEDS_LINUX_CONFIG)
> >
> > And finally in linux/linux.mk:
> >
> > define LINUX_CHECK_CONFIG_OPTS
> > $(foreach cfg,$(strip $(LINUX_NEEDS_CONFIG_OPTS)),\
> > if ! grep -E '^$(cfg)=y$$' $(@D)/.config >/dev/null; then \
> > printf "ERROR: Enable %s in your linux kernel config." "$(cfg)"; \
> > exit 1; \
> > fi;)
> > endef
> > LINUX_POST_CONFIGURE_HOOKS += LINUX_CHECK_CONFIG_OPTS
>
> I agree on the principle, but I think we should rather handle that like
> we do for other options: enable automatically the needed option.
Except for PCI it does not really makes sense, see Arnout's explanations:
http://lists.busybox.net/pipermail/buildroot/2015-October/141178.html
> Remember how someone proposed to check for CONFIG_MODULES=y and error
> out if not available? Instead, you implemented
> b7c32c98bc810b88e0391117f225658f82b44995.
Meh... ;-)
> So a little bit in the direction of what you proposed, what about
> refactoring the LINUX_NEEDS_MODULES mechanism into a more generic
> mechanism that allows package to express which kernel option(s) they
> need, and automatically enable such options in linux/linux.mk when
> configuring the kernel.
I'm not totally oposed to this idea.
However, there are cases, like PCI, for which it does not make sense to
just enable it (as Arnout explained).
Unless we don't care about runtime consistency, and just care about the
buildability of the configuration...
However, I like doing such little tiny bit of infra, so I can append that
to my todo-list. What do you prefer: setting the option (like for
modules), or checking the option?
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 11+ messages in thread* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-12 21:16 ` Yann E. MORIN
@ 2015-10-12 21:24 ` Thomas Petazzoni
2015-10-13 7:03 ` Arnout Vandecappelle
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2015-10-12 21:24 UTC (permalink / raw)
To: buildroot
Dear Yann E. MORIN,
On Mon, 12 Oct 2015 23:16:18 +0200, Yann E. MORIN wrote:
> > I agree on the principle, but I think we should rather handle that like
> > we do for other options: enable automatically the needed option.
>
> Except for PCI it does not really makes sense, see Arnout's explanations:
> http://lists.busybox.net/pipermail/buildroot/2015-October/141178.html
Except that I disagree with Arnout :)
Arnout is not making the difference between being able to *build* and
being able to *run*.
CONFIG_PCI is needed for the build to proceed, so it is normal that we
enable it automatically. The PCI root complex drivers are only needed
for the PCI support to actually work on the target, not for iqvlinux to
build.
And the story is the same for xtable-addons: the additions in linux.mk
were made because those kernel options were needed for xtable-addons to
simply *build*.
> > Remember how someone proposed to check for CONFIG_MODULES=y and error
> > out if not available? Instead, you implemented
> > b7c32c98bc810b88e0391117f225658f82b44995.
>
> Meh... ;-)
No, your commit is good I believe.
> I'm not totally oposed to this idea.
>
> However, there are cases, like PCI, for which it does not make sense to
> just enable it (as Arnout explained).
See above :)
> Unless we don't care about runtime consistency, and just care about the
> buildability of the configuration...
>
> However, I like doing such little tiny bit of infra, so I can append that
> to my todo-list. What do you prefer: setting the option (like for
> modules), or checking the option?
Setting the option, when it is needed to build the external modules.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-12 21:24 ` Thomas Petazzoni
@ 2015-10-13 7:03 ` Arnout Vandecappelle
2015-10-15 19:39 ` Peter Korsgaard
0 siblings, 1 reply; 11+ messages in thread
From: Arnout Vandecappelle @ 2015-10-13 7:03 UTC (permalink / raw)
To: buildroot
On 12-10-15 23:24, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Mon, 12 Oct 2015 23:16:18 +0200, Yann E. MORIN wrote:
>
>>> > > I agree on the principle, but I think we should rather handle that like
>>> > > we do for other options: enable automatically the needed option.
>> >
>> > Except for PCI it does not really makes sense, see Arnout's explanations:
>> > http://lists.busybox.net/pipermail/buildroot/2015-October/141178.html
> Except that I disagree with Arnout :)
>
> Arnout is not making the difference between being able to *build* and
> being able to *run*.
>
> CONFIG_PCI is needed for the build to proceed, so it is normal that we
> enable it automatically. The PCI root complex drivers are only needed
> for the PCI support to actually work on the target, not for iqvlinux to
> build.
>
> And the story is the same for xtable-addons: the additions in linux.mk
> were made because those kernel options were needed for xtable-addons to
> simply *build*.
And of course I disagree with Thomas :-)
I think we should follow the principle of least surprise. We should avoid
situations where a package builds and we know for sure it will not work at
runtime. In many cases, this cannot be avoided, but if we can avoid it, we should.
With xtables it's slightly different since if we enable those kernel config
options, it will actually work at runtime (at least as far as we can tell).
Similar for the systemd options - which BTW are really runtime-only, systemd
doesn't check for them at all at build time.
Fortunately, what you've committed amounts to a build-time check: the iqvlinux
build will error out if CONFIG_PCI isn't enabled. The error message will be
cryptic and will not tell the user how to fix it, but at least it is less
cryptic than when the user enables the package and it simply doesn't do anything
without any error message at all.
So basically, the check added by Romain is just making the build error message
less cryptic, so it is really optional.
Regards,
Arnout
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-13 7:03 ` Arnout Vandecappelle
@ 2015-10-15 19:39 ` Peter Korsgaard
0 siblings, 0 replies; 11+ messages in thread
From: Peter Korsgaard @ 2015-10-15 19:39 UTC (permalink / raw)
To: buildroot
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
Hi,
>> And the story is the same for xtable-addons: the additions in linux.mk
>> were made because those kernel options were needed for xtable-addons to
>> simply *build*.
> And of course I disagree with Thomas :-)
Heh ;)
> I think we should follow the principle of least surprise. We should avoid
> situations where a package builds and we know for sure it will not work at
> runtime. In many cases, this cannot be avoided, but if we can avoid it, we should.
It's not really that we *KNOW* that it won't build, it is more that we
cannot *GUARANTEE* that it will.
For this specific case, I think the issue is moot as people using this
package are most likely on x86 (where having a kernel config without PCI
is very unlikely), or atleast on a platform with PCI support enabled
(otherwise they cannot use the Intel MAC).
But yeah, I agree that we should try to make things work out of the box
(both build and runtime) where possible.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-10 15:04 ` Yann E. MORIN
2015-10-12 20:44 ` Thomas Petazzoni
@ 2015-10-12 21:16 ` Thomas Petazzoni
2015-10-12 21:24 ` Yann E. MORIN
1 sibling, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2015-10-12 21:16 UTC (permalink / raw)
To: buildroot
Yann,
On Sat, 10 Oct 2015 17:04:57 +0200, Yann E. MORIN wrote:
> > +IQVLINUX_MODULE_MAKE_OPTS = NALDIR=$(@D) KSRC=$(LINUX_DIR) CC=$(TARGET_CC)
>
> Are you sure CC is needed? The kernel buildsystem should already set it, no?
Yes, it is really needed. The kernel buildsystem indeed defines CC, but
it gets re-defined by the iqvlinux Makefile, which gets included by the
kernel buildsystem since we do M=/path/to/iqvlinux to build the
iqvlinux module.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-12 21:16 ` Thomas Petazzoni
@ 2015-10-12 21:24 ` Yann E. MORIN
0 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2015-10-12 21:24 UTC (permalink / raw)
To: buildroot
Thomas, Romain, All,
On 2015-10-12 23:16 +0200, Thomas Petazzoni spake thusly:
> On Sat, 10 Oct 2015 17:04:57 +0200, Yann E. MORIN wrote:
> > > +IQVLINUX_MODULE_MAKE_OPTS = NALDIR=$(@D) KSRC=$(LINUX_DIR) CC=$(TARGET_CC)
> >
> > Are you sure CC is needed? The kernel buildsystem should already set it, no?
>
> Yes, it is really needed. The kernel buildsystem indeed defines CC, but
> it gets re-defined by the iqvlinux Makefile, which gets included by the
> kernel buildsystem since we do M=/path/to/iqvlinux to build the
> iqvlinux module.
OK, maybe that could have been explained either as a comment in the .mk
or in the commit log. ;-)
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-09 13:26 [Buildroot] [PATCH v3] package/iqvlinux: new package Romain Naour
2015-10-10 15:04 ` Yann E. MORIN
@ 2015-10-12 21:17 ` Thomas Petazzoni
2015-10-13 9:00 ` Romain Naour
1 sibling, 1 reply; 11+ messages in thread
From: Thomas Petazzoni @ 2015-10-12 21:17 UTC (permalink / raw)
To: buildroot
Dear Romain Naour,
On Fri, 9 Oct 2015 15:26:02 +0200, Romain Naour wrote:
> The PCI support needs to be checked since this driver is based
> on it. Otherwise the build fail with:
> #error "This driver requires PCI support to be available"
>
> But this message is concealed by several occurrence of this
> one:
> error: implicit declaration of function 'pci_find_bus' [-Werror=implicit-function-declaration]
>
> Signed-off-by: Romain Naour <romain.naour@openwide.fr>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
> v3: changes suggested by Arnout
> - remove the patch and use CC=$(TARGET_CC) instead.
> - Add a 'or' between GPLv2 and BSD-3c for license information
> - Add some more license files (files.txt)
> - Fixes typos and indentation.
> - move the check for PCI option to LINUX_POST_CONFIGURE_HOOKS
> - redirect echo to stderr.
> v2: - rename the package simply to iqvlinux (ThomasP)
> - move it to "Hardware Handling" menu (ThomasP)
> - Cc Yann for the kernel-module infra
> - Add a check for CONFIG_PCI even if it's redundant with
> the message from the Makefile.
> (Do we really need this check ?)
> ---
> package/Config.in | 1 +
> package/iqvlinux/Config.in | 18 ++++++++++++++++++
> package/iqvlinux/iqvlinux.hash | 5 +++++
> package/iqvlinux/iqvlinux.mk | 34 ++++++++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+)
> create mode 100644 package/iqvlinux/Config.in
> create mode 100644 package/iqvlinux/iqvlinux.hash
> create mode 100644 package/iqvlinux/iqvlinux.mk
Applied after doing the following changes:
[Thomas:
- fix minor typo in Config.in: s/Enthernet/Ethernet/
- license is "GPLv2, BSD-3c", not "GPLv2 or BSD-3c"
- remove IQVLINUX_PCI_CHECK, until a proper generic solution is
implemented.]
As you can see, I've removed the IQVLINUX_PCI_CHECK so that a nicer
solution can be discussed/implemented with Yann. However, since this
check was not really mandatory, I decided to add the package
nonetheless (albeit without the check).
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 11+ messages in thread* [Buildroot] [PATCH v3] package/iqvlinux: new package
2015-10-12 21:17 ` Thomas Petazzoni
@ 2015-10-13 9:00 ` Romain Naour
0 siblings, 0 replies; 11+ messages in thread
From: Romain Naour @ 2015-10-13 9:00 UTC (permalink / raw)
To: buildroot
Hi Thomas,
Le 12/10/2015 23:17, Thomas Petazzoni a ?crit :
> Dear Romain Naour,
>
> On Fri, 9 Oct 2015 15:26:02 +0200, Romain Naour wrote:
>> The PCI support needs to be checked since this driver is based
>> on it. Otherwise the build fail with:
>> #error "This driver requires PCI support to be available"
>>
>> But this message is concealed by several occurrence of this
>> one:
>> error: implicit declaration of function 'pci_find_bus' [-Werror=implicit-function-declaration]
>>
>> Signed-off-by: Romain Naour <romain.naour@openwide.fr>
>> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>> ---
>> v3: changes suggested by Arnout
>> - remove the patch and use CC=$(TARGET_CC) instead.
>> - Add a 'or' between GPLv2 and BSD-3c for license information
>> - Add some more license files (files.txt)
>> - Fixes typos and indentation.
>> - move the check for PCI option to LINUX_POST_CONFIGURE_HOOKS
>> - redirect echo to stderr.
>> v2: - rename the package simply to iqvlinux (ThomasP)
>> - move it to "Hardware Handling" menu (ThomasP)
>> - Cc Yann for the kernel-module infra
>> - Add a check for CONFIG_PCI even if it's redundant with
>> the message from the Makefile.
>> (Do we really need this check ?)
>> ---
>> package/Config.in | 1 +
>> package/iqvlinux/Config.in | 18 ++++++++++++++++++
>> package/iqvlinux/iqvlinux.hash | 5 +++++
>> package/iqvlinux/iqvlinux.mk | 34 ++++++++++++++++++++++++++++++++++
>> 4 files changed, 58 insertions(+)
>> create mode 100644 package/iqvlinux/Config.in
>> create mode 100644 package/iqvlinux/iqvlinux.hash
>> create mode 100644 package/iqvlinux/iqvlinux.mk
>
> Applied after doing the following changes:
>
> [Thomas:
> - fix minor typo in Config.in: s/Enthernet/Ethernet/
> - license is "GPLv2, BSD-3c", not "GPLv2 or BSD-3c"
> - remove IQVLINUX_PCI_CHECK, until a proper generic solution is
> implemented.]
>
> As you can see, I've removed the IQVLINUX_PCI_CHECK so that a nicer
> solution can be discussed/implemented with Yann. However, since this
> check was not really mandatory, I decided to add the package
> nonetheless (albeit without the check).
Ok, it's not a problem for me. The kernel I use already have CONFIG_PCI enabled.
I'll follow the discussion about the kernel option check.
Best regards,
Romain
>
> Thanks!
>
> Thomas
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-10-15 19:39 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-09 13:26 [Buildroot] [PATCH v3] package/iqvlinux: new package Romain Naour
2015-10-10 15:04 ` Yann E. MORIN
2015-10-12 20:44 ` Thomas Petazzoni
2015-10-12 21:16 ` Yann E. MORIN
2015-10-12 21:24 ` Thomas Petazzoni
2015-10-13 7:03 ` Arnout Vandecappelle
2015-10-15 19:39 ` Peter Korsgaard
2015-10-12 21:16 ` Thomas Petazzoni
2015-10-12 21:24 ` Yann E. MORIN
2015-10-12 21:17 ` Thomas Petazzoni
2015-10-13 9:00 ` Romain Naour
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox