Skip to contentSkip to conversation

spidev_test: use the one from the kernel we build

PATCH New Vincent Stehlé 3 participants 5 messages Jan 19, 2015 – Jul 12, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 spidev_test: use the one from the kernel we build

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.

Cc Gustavo Zacarias <[email protected]>
Cc Thomas Petazzoni <[email protected]>

---

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(-)
package/spidev_test/Config.in +11 -0
index 0d8214c..8da096e 100644
@@ -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
package/spidev_test/spidev_test.mk +12 -1
index 646372f..5ab80a6 100644
@@ -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
Signature · 1 line
2.1.4
#1 lore raw

PATCH 1/1 spidev_test: use the one from the kernel we build

Vincent, All,
On 2015-01-19 20:35 +0100, Vincent Stehl? spake thusly:

Quoted 3 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#2 lore raw

PATCH 1/1 spidev_test: use the one from the kernel we build

Vincent, All,

On 2015-01-19 22:13 +0100, Yann E. MORIN spake thusly:

Quoted 7 lines

Vincent, All,
On 2015-01-19 20:35 +0100, Vincent Stehl? spake thusly:

Quoted 3 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#3 lore raw

PATCH 1/1 spidev_test: use the one from the kernel we build

On 01/19/2015 10:32 PM, Yann E. MORIN wrote:
..

Quoted 1 line

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.

#4 lore raw

PATCH 1/1 spidev_test: use the one from the kernel we build

In reply to Vincent Stehlé

Hi Vincent,

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

Quoted 3 lines · 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.

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

Quoted 4 lines
Cc Gustavo Zacarias <[email protected]>
Cc Thomas Petazzoni <[email protected]>

[snip]

Signature · 6 lines
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