[Buildroot] [PATCH] spidev_test: new package

[Buildroot] [PATCH] spidev_test: new package

[Gustavo Zacarias]

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/Config.in                  |  1 +
 package/spidev_test/Config.in      | 15 +++++++++++++++
 package/spidev_test/spidev_test.mk | 22 ++++++++++++++++++++++
 3 files changed, 38 insertions(+)
 create mode 100644 package/spidev_test/Config.in
 create mode 100644 package/spidev_test/spidev_test.mk

diff --git a/package/Config.in b/package/Config.in
index dda7716..fb8e93d 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -84,6 +84,7 @@ endif
 	source "package/ramsmp/Config.in"
 	source "package/ramspeed/Config.in"
 	source "package/rt-tests/Config.in"
+	source "package/spidev_test/Config.in"
 	source "package/strace/Config.in"
 	source "package/stress/Config.in"
 	source "package/sysprof/Config.in"
diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
new file mode 100644
index 0000000..c0ed9ef
--- /dev/null
+++ b/package/spidev_test/Config.in
@@ -0,0 +1,15 @@
+config BR2_PACKAGE_SPIDEV_TEST
+	bool "spidev_test"
+	depends on BR2_LINUX_KERNEL
+	help
+	  SPI testing utility (using spidev driver).
+
+	  This package builds and installs the userspace 'spidev_test'
+	  command. It is up to the user to ensure that the kernel
+	  configuration has all the suitable options enabled to allow a
+	  proper operation of 'spidev_test'.
+
+	  https://www.kernel.org/doc/Documentation/spi/spidev_test.c
+
+comment "spidev_test needs Linux kernel to be built"
+	depends on !BR2_LINUX_KERNEL
diff --git a/package/spidev_test/spidev_test.mk b/package/spidev_test/spidev_test.mk
new file mode 100644
index 0000000..24c0f89
--- /dev/null
+++ b/package/spidev_test/spidev_test.mk
@@ -0,0 +1,22 @@
+################################################################################
+#
+# spidev_test
+#
+################################################################################
+
+# Source taken from the Linux kernel tree
+SPIDEV_TEST_SOURCE =
+SPIDEV_TEST_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
+SPIDEV_TEST_DEPENDENCIES = linux
+
+define SPIDEV_TEST_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(TARGET_CC) $(TARGET_CFLAGS) -o $(@D)/spidev_test \
+		$(LINUX_DIR)/Documentation/spi/spidev_test.c
+endef
+
+define SPIDEV_TEST_INSTALL_TARGET_CMDS
+	$(INSTALL) -D -m 755 $(@D)/spidev_test \
+		$(TARGET_DIR)/usr/sbin/spidev_test
+endef
+
+$(eval $(generic-package))
-- 
2.0.4

[Buildroot] [PATCH] spidev_test: new package

[Thomas De Schampheleire]

Gustavo Zacarias <gustavo@zacarias.com.ar> schreef:
>Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
>---
> package/Config.in                  |  1 +
> package/spidev_test/Config.in      | 15 +++++++++++++++
> package/spidev_test/spidev_test.mk | 22 ++++++++++++++++++++++
> 3 files changed, 38 insertions(+)
> create mode 100644 package/spidev_test/Config.in
> create mode 100644 package/spidev_test/spidev_test.mk
>
>diff --git a/package/Config.in b/package/Config.in
>index dda7716..fb8e93d 100644
>--- a/package/Config.in
>+++ b/package/Config.in
>@@ -84,6 +84,7 @@ endif
> 	source "package/ramsmp/Config.in"
> 	source "package/ramspeed/Config.in"
> 	source "package/rt-tests/Config.in"
>+	source "package/spidev_test/Config.in"
> 	source "package/strace/Config.in"
> 	source "package/stress/Config.in"
> 	source "package/sysprof/Config.in"
>diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
>new file mode 100644
>index 0000000..c0ed9ef
>--- /dev/null
>+++ b/package/spidev_test/Config.in
>@@ -0,0 +1,15 @@
>+config BR2_PACKAGE_SPIDEV_TEST
>+	bool "spidev_test"
>+	depends on BR2_LINUX_KERNEL
>+	help
>+	  SPI testing utility (using spidev driver).
>+
>+	  This package builds and installs the userspace 'spidev_test'
>+	  command. It is up to the user to ensure that the kernel
>+	  configuration has all the suitable options enabled to allow a
>+	  proper operation of 'spidev_test'.
>+
>+	  https://www.kernel.org/doc/Documentation/spi/spidev_test.c
>+
>+comment "spidev_test needs Linux kernel to be built"

This should be '... needs a Linux ...'

>+	depends on !BR2_LINUX_KERNEL
>diff --git a/package/spidev_test/spidev_test.mk b/package/spidev_test/spidev_test.mk
>new file mode 100644
>index 0000000..24c0f89
>--- /dev/null
>+++ b/package/spidev_test/spidev_test.mk
>@@ -0,0 +1,22 @@
>+################################################################################
>+#
>+# spidev_test
>+#
>+################################################################################
>+
>+# Source taken from the Linux kernel tree
>+SPIDEV_TEST_SOURCE =
>+SPIDEV_TEST_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
>+SPIDEV_TEST_DEPENDENCIES = linux

Here I think we should define the license and if possible license files, if needed using a hook to copy the license from the kernel...

>+
>+define SPIDEV_TEST_BUILD_CMDS
>+	$(TARGET_MAKE_ENV) $(TARGET_CC) $(TARGET_CFLAGS) -o $(@D)/spidev_test \
>+		$(LINUX_DIR)/Documentation/spi/spidev_test.c
>+endef
>+
>+define SPIDEV_TEST_INSTALL_TARGET_CMDS
>+	$(INSTALL) -D -m 755 $(@D)/spidev_test \
>+		$(TARGET_DIR)/usr/sbin/spidev_test
>+endef
>+
>+$(eval $(generic-package))

Good idea to make this a package. We once copied this test to our own test repo, but this makes more sense.

Best regards,
Thomas

[Buildroot] [PATCH] spidev_test: new package

[Gustavo Zacarias]

On 10/14/2014 02:29 PM, Thomas De Schampheleire wrote:
>> +comment "spidev_test needs Linux kernel to be built"
> 
> This should be '... needs a Linux ...'

I'll fix it.

> Here I think we should define the license and if possible license files, if needed using a hook to copy the license from the kernel...

Well the kernel & linux-headers aren't defining them either :)
And perf (another kernel-source based package) isn't either.

> Good idea to make this a package. We once copied this test to our own test repo, but this makes more sense.

This is a "fragile" package though, a new enough kernel version against
older headers might not build because of missing spidev defines.
On the autobuilder side that's no problem since it depends on the kernel
being built which for the time being isn't the case, but it should
probably be noted as such.

Regards.

[Buildroot] [PATCH] spidev_test: new package

[Ryan Barnett]

Gustavo,

On Tue, Oct 14, 2014 at 10:45 AM, Gustavo Zacarias
<gustavo@zacarias.com.ar> wrote:
> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> ---
>  package/Config.in                  |  1 +
>  package/spidev_test/Config.in      | 15 +++++++++++++++
>  package/spidev_test/spidev_test.mk | 22 ++++++++++++++++++++++
>  3 files changed, 38 insertions(+)
>  create mode 100644 package/spidev_test/Config.in
>  create mode 100644 package/spidev_test/spidev_test.mk

I did this exact same thing but never got around to submitting this.
This is a very handy tool for checking out SPI devices from command
line.

Reviewed-by: Ryan Barnett <ryan.barnett@rockwellcollins.com>

[Buildroot] [PATCH] spidev_test: new package

[Baruch Siach]

Hi Gustavo,

On Tue, Oct 14, 2014 at 12:45:53PM -0300, Gustavo Zacarias wrote:
> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>

[snip]

> diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
> new file mode 100644
> index 0000000..c0ed9ef
> --- /dev/null
> +++ b/package/spidev_test/Config.in
> @@ -0,0 +1,15 @@
> +config BR2_PACKAGE_SPIDEV_TEST
> +	bool "spidev_test"
> +	depends on BR2_LINUX_KERNEL

That is a heavy dependency. As you noted in another message, it also make the 
autobuilder skip this package. I suggest to download just spidev_test.c from 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). This 
solves both the autobuilder issue, and the toolchain headers dependency.

baruch

> +	help
> +	  SPI testing utility (using spidev driver).
> +
> +	  This package builds and installs the userspace 'spidev_test'
> +	  command. It is up to the user to ensure that the kernel
> +	  configuration has all the suitable options enabled to allow a
> +	  proper operation of 'spidev_test'.
> +
> +	  https://www.kernel.org/doc/Documentation/spi/spidev_test.c
> +
> +comment "spidev_test needs Linux kernel to be built"
> +	depends on !BR2_LINUX_KERNEL
> diff --git a/package/spidev_test/spidev_test.mk b/package/spidev_test/spidev_test.mk
> new file mode 100644
> index 0000000..24c0f89
> --- /dev/null
> +++ b/package/spidev_test/spidev_test.mk
> @@ -0,0 +1,22 @@
> +################################################################################
> +#
> +# spidev_test
> +#
> +################################################################################
> +
> +# Source taken from the Linux kernel tree
> +SPIDEV_TEST_SOURCE =
> +SPIDEV_TEST_VERSION = $(call qstrip,$(BR2_LINUX_KERNEL_VERSION))
> +SPIDEV_TEST_DEPENDENCIES = linux
> +
> +define SPIDEV_TEST_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(TARGET_CC) $(TARGET_CFLAGS) -o $(@D)/spidev_test \
> +		$(LINUX_DIR)/Documentation/spi/spidev_test.c
> +endef
> +
> +define SPIDEV_TEST_INSTALL_TARGET_CMDS
> +	$(INSTALL) -D -m 755 $(@D)/spidev_test \
> +		$(TARGET_DIR)/usr/sbin/spidev_test
> +endef
> +
> +$(eval $(generic-package))

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH] spidev_test: new package

[Gustavo Zacarias]

On 10/14/2014 04:54 PM, Baruch Siach wrote:

> That is a heavy dependency. As you noted in another message, it also make the 
> autobuilder skip this package. I suggest to download just spidev_test.c from 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
> and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). This 
> solves both the autobuilder issue, and the toolchain headers dependency.
> 
> baruch

Hi Baruch.
Problem is that makes it need a newish kernel which i don't like since
you can't always run the latest and greatest.
For instance i'm using it on a 3.10-ish based kernel at the moment and
if i did that i wouldn't be able to use it.
I suspect the same logic will apply to other users as well.
Regards.

[Buildroot] [PATCH] spidev_test: new package

[Baruch Siach]

Hi Gustavo,

On Tue, Oct 14, 2014 at 04:56:38PM -0300, Gustavo Zacarias wrote:
> On 10/14/2014 04:54 PM, Baruch Siach wrote:
> > That is a heavy dependency. As you noted in another message, it also make 
> > the autobuilder skip this package. I suggest to download just 
> > spidev_test.c from 
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
> > and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). 
> > This solves both the autobuilder issue, and the toolchain headers 
> > dependency.
> 
> Problem is that makes it need a newish kernel which i don't like since
> you can't always run the latest and greatest.
> For instance i'm using it on a 3.10-ish based kernel at the moment and
> if i did that i wouldn't be able to use it.
> I suspect the same logic will apply to other users as well.

Then maybe adding a version choice would be best. spidev_test.c didn't change 
between v2.6.34 and v3.15, so two versions should be enough.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH] spidev_test: new package

[Thomas De Schampheleire]

Baruch Siach <baruch@tkos.co.il> schreef:
>Hi Gustavo,
>
>On Tue, Oct 14, 2014 at 12:45:53PM -0300, Gustavo Zacarias wrote:
>> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
>
>[snip]
>
>> diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
>> new file mode 100644
>> index 0000000..c0ed9ef
>> --- /dev/null
>> +++ b/package/spidev_test/Config.in
>> @@ -0,0 +1,15 @@
>> +config BR2_PACKAGE_SPIDEV_TEST
>> +	bool "spidev_test"
>> +	depends on BR2_LINUX_KERNEL
>
>That is a heavy dependency. As you noted in another message, it also make the 
>autobuilder skip this package. I suggest to download just spidev_test.c from 
>http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
>and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). This 
>solves both the autobuilder issue, and the toolchain headers dependency.

From where comes this dependency on 3.15/3.17?

The test program was in the kernel for a very long time (2.6.x)...

Also, what is the autobuilder issue you are talking about?

[Buildroot] [PATCH] spidev_test: new package

[Gustavo Zacarias]

On 10/14/2014 05:14 PM, Thomas De Schampheleire wrote:
>> That is a heavy dependency. As you noted in another message, it also make the 
>> autobuilder skip this package. I suggest to download just spidev_test.c from 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
>> and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). This 
>> solves both the autobuilder issue, and the toolchain headers dependency.
> 
> From where comes this dependency on 3.15/3.17?
> 
> The test program was in the kernel for a very long time (2.6.x)...
> 
> Also, what is the autobuilder issue you are talking about?

SPI_RX_QUAD/SPI_TX_QUAD (quad-pumped spi).
The autobuilder issue is that older kernel headers won't have this
defined (example: external toolchains with newish kernel).
What Baruch proposes is probably the best solution, otherwise a ton of
#ifdefs is option 2 which would probably wouldn't be acceptable upstream.
Regards.

[Buildroot] [PATCH] spidev_test: new package

[Baruch Siach]

Hi Thomas,

On Tue, Oct 14, 2014 at 10:14:57PM +0200, Thomas De Schampheleire wrote:
> Baruch Siach <baruch@tkos.co.il> schreef:
> >On Tue, Oct 14, 2014 at 12:45:53PM -0300, Gustavo Zacarias wrote:
> >> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> >
> >[snip]
> >
> >> diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
> >> new file mode 100644
> >> index 0000000..c0ed9ef
> >> --- /dev/null
> >> +++ b/package/spidev_test/Config.in
> >> @@ -0,0 +1,15 @@
> >> +config BR2_PACKAGE_SPIDEV_TEST
> >> +	bool "spidev_test"
> >> +	depends on BR2_LINUX_KERNEL
> >
> >That is a heavy dependency. As you noted in another message, it also make the 
> >autobuilder skip this package. I suggest to download just spidev_test.c from 
> >http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
> >and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). This 
> >solves both the autobuilder issue, and the toolchain headers dependency.
> 
> From where comes this dependency on 3.15/3.17?

Linux kernel commit c2e78c34ef0 (spi: spidev_test: Add support for Dual/Quad 
SPI Transfers) added a build dependency on SPI_IOC_{RD,WR}_MODE32 which is 
only available since v3.15.

> The test program was in the kernel for a very long time (2.6.x)...
> 
> Also, what is the autobuilder issue you are talking about?

A dependency on BR2_LINUX_KERNEL means that the autobuilder won't build this 
package, since the autobuilder does not build kernels, and so never enables 
BR2_LINUX_KERNEL.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH] spidev_test: new package

[Yann E. MORIN]

Baruch, All,

On 2014-10-14 22:54 +0300, Baruch Siach spake thusly:
> On Tue, Oct 14, 2014 at 12:45:53PM -0300, Gustavo Zacarias wrote:
> > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> 
> [snip]
> 
> > diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
> > new file mode 100644
> > index 0000000..c0ed9ef
> > --- /dev/null
> > +++ b/package/spidev_test/Config.in
> > @@ -0,0 +1,15 @@
> > +config BR2_PACKAGE_SPIDEV_TEST
> > +	bool "spidev_test"
> > +	depends on BR2_LINUX_KERNEL
> 
> That is a heavy dependency. As you noted in another message, it also make the 
> autobuilder skip this package. I suggest to download just spidev_test.c from 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
> and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). This 

What about directly getting the appropriate one, something like:

    ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15),y)
    SPIDEV_TEST_VERSION = v3.15
    else
    SPIDEV_TEST_VERSION = v3.14
    endif

    SPIDEV_TEST_SOURCE =
    SPIDEV_TEST_EXTRA_DOWNLOADS = http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION)

Since, without the proper kernel headers, it is not possible to build
the quad-pump blabla, we can directly get the spidev_test that will
indeed compile with our toolchain, no?

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: new package

[Baruch Siach]

Hi Yann,

On Fri, Oct 17, 2014 at 11:55:39PM +0200, Yann E. MORIN wrote:
> On 2014-10-14 22:54 +0300, Baruch Siach spake thusly:
> > On Tue, Oct 14, 2014 at 12:45:53PM -0300, Gustavo Zacarias wrote:
> > > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> > 
> > [snip]
> > 
> > > diff --git a/package/spidev_test/Config.in b/package/spidev_test/Config.in
> > > new file mode 100644
> > > index 0000000..c0ed9ef
> > > --- /dev/null
> > > +++ b/package/spidev_test/Config.in
> > > @@ -0,0 +1,15 @@
> > > +config BR2_PACKAGE_SPIDEV_TEST
> > > +	bool "spidev_test"
> > > +	depends on BR2_LINUX_KERNEL
> > 
> > That is a heavy dependency. As you noted in another message, it also make the 
> > autobuilder skip this package. I suggest to download just spidev_test.c from 
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION), 
> > and make it depend on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 (as of v3.17). This 
> 
> What about directly getting the appropriate one, something like:
> 
>     ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15),y)
>     SPIDEV_TEST_VERSION = v3.15
>     else
>     SPIDEV_TEST_VERSION = v3.14
>     endif
> 
>     SPIDEV_TEST_SOURCE =
>     SPIDEV_TEST_EXTRA_DOWNLOADS = http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/spi/spidev_test.c?id=$(SPIDEV_TEST_VERSION)
> 
> Since, without the proper kernel headers, it is not possible to build
> the quad-pump blabla, we can directly get the spidev_test that will
> indeed compile with our toolchain, no?

As Gustavo mentioned in another message in this thread, the newer spidev_test 
is also run-time dependent on kernel v3.15 or newer. An automatic spidev_test 
version choice would produce a broken binary for users of newer toolchains 
with pre v3.15 kernels. We should use BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_15 to 
disable build of v3.15 spidev_test, as this would break the build. But I don't 
think we want to force v3.15 spidev_test just because the toolchain kernel 
headers are post v3.15.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -