[Buildroot] [PATCH] spidev_test: use the one from the kernel we build

[Buildroot] [PATCH] spidev_test: use the one from the kernel we build

[Vincent Stehlé]

Add a configuration option to use the spidev_test.c from the Linux kernel we
build. This allows to be perfectly consistent between spidev_test and the
kernel in use.

Signed-off-by: Vincent Stehl? <vincent.stehle@freescale.com>
Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---

Hi,

Do not hesitate to let me know if you don't like the variables names; for
example BR2_PACKAGE_SPIDEV_TEST_FROM_KERNEL and SPIDEV_TEST_C_FILE_SRC.

Best regards,

V.

 package/spidev_test/Config.in      | 11 +++++++++++
 package/spidev_test/spidev_test.mk | 13 ++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
index 0d8214c..8da096e 100644
--- a/package/spidev_test/Config.in
+++ b/package/spidev_test/Config.in
@@ -15,3 +15,14 @@ config BR2_PACKAGE_SPIDEV_TEST
 	  toolchain is too old.
 
 	  https://www.kernel.org/doc/Documentation/spi/spidev_test.c
+
+if BR2_PACKAGE_SPIDEV_TEST
+
+config BR2_PACKAGE_SPIDEV_TEST_FROM_KERNEL
+	bool "spidev_test.c of the linux kernel we build"
+	depends on BR2_LINUX_KERNEL
+	help
+	  Use directly the spidev_test.c from the kernel sources we build,
+	  instead of retrieving from kernel.org.
+
+endif
diff --git a/package/spidev_test/spidev_test.mk b/package/spidev_test/spidev_test.mk
index 646372f..5ab80a6 100644
--- a/package/spidev_test/spidev_test.mk
+++ b/package/spidev_test/spidev_test.mk
@@ -4,6 +4,14 @@
 #
 ################################################################################
 
+ifeq ($(BR2_PACKAGE_SPIDEV_TEST_FROM_KERNEL),y)
+# Use the spidev_test.c from the kernel we build.
+SPIDEV_TEST_SOURCE =
+SPIDEV_TEST_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
+SPIDEV_TEST_DEPENDENCIES = linux
+SPIDEV_TEST_C_FILE_SRC = $(LINUX_DIR)/Documentation/spi/spidev_test.c
+else
+# Get spidev_test.c from kernel.org.
 # v3.15+ requires SPI_TX_QUAD/SPI_RX_QUAD to build
 # Normally kernel headers can't be newer than kernel so switch based on that.
 # If you need quad-pumped spi support you need to upgrade your toolchain.
@@ -14,10 +22,13 @@ SPIDEV_TEST_VERSION = v3.0
 endif
 SPIDEV_TEST_SITE = http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi
 SPIDEV_TEST_SOURCE = spidev_test.c?id=$(SPIDEV_TEST_VERSION)
+SPIDEV_TEST_C_FILE_SRC = $(BR2_DL_DIR)/$(SPIDEV_TEST_SOURCE)
+endif
+
 SPIDEV_TEST_LICENSE = GPLv2
 
 define SPIDEV_TEST_EXTRACT_CMDS
-	cp $(BR2_DL_DIR)/$(SPIDEV_TEST_SOURCE) $(@D)/spidev_test.c
+	cp $(SPIDEV_TEST_C_FILE_SRC) $(@D)/spidev_test.c
 endef
 
 define SPIDEV_TEST_BUILD_CMDS
-- 
2.1.4

[Buildroot] [PATCH] spidev_test: use the one from the kernel we build

[Yann E. MORIN]

Vincent, All,
On 2015-01-19 20:35 +0100, Vincent Stehl? spake thusly:
> Add a configuration option to use the spidev_test.c from the Linux kernel we
> build. This allows to be perfectly consistent between spidev_test and the
> kernel in use.

So, let me try to summarise: we want spidev_test to be exactly in line
with the code from the Linux kernel, because using an older spidev_test
on a newr kernel (or the other way around) is just pointless because it
would not run, right?

But looking at the code, it seems 3.15 seems to be the pivot. any combo
before it would work, every combo after would, not just mix before/after
3.15, right?

Finally, it seems we are (currently) only interested on the heders used
for the toolchain, not the actual kernel running (but that's just
looking at out spidev_test.mk). Is there really no run-time dependency?


Now, say I have a toolchain with 3.15-or-later headers: Buildroot would
build the 3.15-or-later spidev_test. But then what if Buildroot is *not*
building a kernel and I build it outside Buildroot and it is a 3.14
kernel? That would just break at runtime, no?

Or the other way around: my toolchain has 3.14-or-older headers
(plausible with a vendor-provided toolchain), but my kernel is a 3.15+.
So Buildroot would build a 3.14-or-older spidev_test, and that would
break on my 3.15-or-later kernel, no?


So I would say, just have our spidev_test package depend on a Linux
kernel being built, always; and copy spidev_test.c from the Linux source
tree, always.

If the user wants to build a kernel outside of Buildroot, it is their
responsibility to build the appropriate spidev_test.

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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] spidev_test: use the one from the kernel we build

[Yann E. MORIN]

Vincent, All,

On 2015-01-19 22:13 +0100, Yann E. MORIN spake thusly:
> Vincent, All,
> On 2015-01-19 20:35 +0100, Vincent Stehl? spake thusly:
> > Add a configuration option to use the spidev_test.c from the Linux kernel we
> > build. This allows to be perfectly consistent between spidev_test and the
> > kernel in use.
> 
> So, let me try to summarise: [...]

After discussing on IRC with Gustavo, I decided to further reply with
some clarifications.

I'll use a table, that should much easier to understand (hopefully).

We need to have spidev_test buildable, so it needs to be aligned with
the kernel headers from the toolchain.

toolchain headers:  3.14-   3.15+
spidev_test:
    3.14-            OK      OK*
    3.15+            KO      OK

* builds but is missing new features.

Ten at runtime we need a similar table:

spidev_test:   3.14-    3.15+
kernel:
    3.14-       OK       KO
    3.15+       OK*      OK

[*] builds and runs, but missing extra features.

The only broken case is when using toolchain headers 3.15+ with a 3.14-
kernel, becuse currently, we'd build a 3.15+ spidev_test, and that would
break at runtime.

Al other cases are supposed to build and run, albeit with restricted
features in one cae: using 3.14- toolchain headers with a 3.15+ Linux
kernel.

(Note: by "toolchain headers X.Y", I really mean "toolchain with kernel
headers version X.Y").

So, we're currently screwed with a too-recent toolchain that builds an
ancient kernel.

Is that all correct?

So, if we want to be bullet-proof, we can *not* build spidev_test if we
do not know both the toolchain headers version *and* the running kernel
version. Otherwise, all we can do is build the largest common
denominator, which is a spidev_test 3.14- (that is, a 3.0 in our current
code).

Anything I missed?

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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] spidev_test: use the one from the kernel we build

[Vincent Stehlé]

On 01/19/2015 10:32 PM, Yann E. MORIN wrote:
..
> Anything I missed?

Hi Yann,

Thanks for your concern.

I am not sure about all those headers and kernel version constrains, sorry.

The point of the proposed patch was more to be able to keep both the
kernel and spidev_test.c in one place, in sync.
  This is especially useful when fiddling into an SPI driver for
example, and adapting the userspace test in the mean time.

Do you think the patch needs to be further modified?

Best regards,

V.

[Buildroot] [PATCH] spidev_test: use the one from the kernel we build

[Arnout Vandecappelle]

 Hi Vincent,

 We're doing a big patchwork cleanup and finally getting back at your patch.

On 01/19/15 20:35, Vincent Stehl? wrote:
> Add a configuration option to use the spidev_test.c from the Linux kernel we
> build. This allows to be perfectly consistent between spidev_test and the
> kernel in use.

 The conclusion of the earlier discussion is that spidev_test should really only
be built together with the kernel, so you could remove the extra config option.
Note, if you do that, there may be a need for legacy handling otherwise a
.config which previously had SPIDEV_TEST but no kernel would suddenly loose the
SPIDEV_TEST.

 However, there is also the new kernel-tools infra proposed by Romain, and this
change would fit into that approach perfectly. So if you resubmit, consider
using the kernel-tools infra.


 Regards,
 Arnout

> 
> Signed-off-by: Vincent Stehl? <vincent.stehle@freescale.com>
> Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
[snip]


-- 
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