cpb- / spi-tools Goto Github PK

So I just commented out "include config.h",
and hardcoded VERSION which allowed make to directly compile.

Autoconf seems to be an extra step that cant find .in files in my case.

The line explaining cross-compilation is wrong. To cross-compile we have to use ./configure option.

I'd like to bump the version in buildroot to include the recent commit of the cflags fixup.

Hi cpb-,

 I found the following bug in the spi-pipe.c you added while fixing  #17.

spi-pipe.c: In function ‘main’:
spi-pipe.c:168:16: error: statement with no effect [-Werror=unused-value]
   transfer.len == offset;
   ----------------------^-------------
cc1: all warnings being treated as errors

The spi-config command accepts some options that spi-pipe does not know.
For example --bits, --mode or --spirdy.
While it is always possible to run spi-config -w in a terminal while using spi-pipe in another terminal, it would be better to have the same configuration options.
Note that -b is already used in spi-pipe for "block size", so we need to use a different short-option, for example -B.

This prevents flags from inheriting the provided CFLAGS value at compile time and instead always uses either "-g -Wall -Werror -Wno-uninitialized -O0" or "$AM_CFLAGS -O0" where $AM_CFLAGS is empty.

I believe a fix would be to make the non-debug case "-O0 CFLAGS" and similar for AM_CXXFLAGS so that a provided optimization would also override the -O0

How come we attempt to write the existing SPI mode again even if no new SPI mode is specified below ?
https://github.com/cpb-/spi-tools/blob/master/src/spi-pipe.c#L195
https://github.com/cpb-/spi-tools/blob/master/src/spi-pipe.c#L220

In my case, this extra SPI configuration writes (4 ioctl calls) doesn't always complete successfully and causing my program to stop.
SPI_IOC_WR_MODE: Device or resource busy

I can do a pull request if you agree this should only be done if new SPI mode is specified.

In man/spi-config.1, the word "idle" is misspelled as "iddle" 4 times (lines 44, 47, 50, 53).

I’am using the
linux: 5.4.0-xilinx-v2020.2
spi-tools: a3f1f68

I have an issue with using the SPI_IOC_RD_LSB_FIRST and SPI_IOC_WR_LSB_FIRST ioctl calls.
With the spi-config –l 1 command I could set this parameter in the kernel spi driver.
But –q query command returns always lsb=0.

I see an unconsistency in the usage of the parameter of these calls in the spi-tools and spidev kernel.

In the spio-tools/spiconfig.c the parameter valid values are : 0, SPI_LSB_FIRST
if (ioctl(fd, SPI_IOC_RD_LSB_FIRST, & byte) < 0) {
perror("SPI_IOC_RD_LSB_FIRST");
exit(EXIT_FAILURE);
}
config.lsb = (byte == SPI_LSB_FIRST ? 1 : 0);

 if ((config.lsb != new_config.lsb) && (new_config.lsb != -1)) {
       byte = (new_config.lsb ? SPI_LSB_FIRST : 0);
       if (ioctl(fd, SPI_IOC_WR_LSB_FIRST, & byte) < 0) {
            perror("SPI_IOC_WR_LSB_FIRST");
            exit(EXIT_FAILURE);
       }
 }

In the spidev.c the valid parameter values are: 0, 1
case SPI_IOC_RD_LSB_FIRST:
retval = put_user((spi->mode & SPI_LSB_FIRST) ? 1 : 0,
(__u8 __user *)arg);
break;

 case SPI_IOC_WR_LSB_FIRST:
       retval = get_user(tmp, (__u8 __user *)arg);
       if (retval == 0) {
            u32  save = spi->mode;

            if (tmp)
                  spi->mode |= SPI_LSB_FIRST;
            else
                  spi->mode &= ~SPI_LSB_FIRST;
            retval = spi_setup(spi);
            if (retval < 0)
                  spi->mode = save;
            else
                  dev_dbg(&spi->dev, "%csb first\n",
                             tmp ? 'l' : 'm');
       }
       break;

I read on the page https://www.kernel.org/doc/Documentation/spi/spidev
"SPI_IOC_RD_LSB_FIRST, SPI_IOC_WR_LSB_FIRST ... pass a pointer to a byte
which will return (RD) or assign (WR) the bit justification used to
transfer SPI words. Zero indicates MSB-first; other values indicate
the less common LSB-first encoding. In both cases the specified value
is right-justified in each word, so that unused (TX) or undefined (RX)
bits are in the MSBs."

I think that modifying the spi-tools SPI_IOC_RD_BITS_PER_WORD not checking against the SPI_LSB_FIRST value, but non zero, we can resolve the issue :
if (ioctl(fd, SPI_IOC_RD_LSB_FIRST, & byte) < 0) {
perror("SPI_IOC_RD_LSB_FIRST");
exit(EXIT_FAILURE);
}
config.lsb = (byte ? 1 : 0);

The use of conftools might be a bit overkill for a codebase this size, and complicates cross compiling (for me at least, I'm not very capable.) I compile this with a simple Makefile using the toolchain OpenWrt gives me after compiling firmware:

TOOLCHAIN = ..//openwrt-toolchain-ath79-generic_gcc-12.3.0_musl.Linux-x86_64/toolchain-mips_24kc_gcc-12.3.0_musl
CC = $(TOOLCHAIN)/bin/mips-openwrt-linux-musl-gcc

default: all

spi-config:
	mkdir -p bin
	echo "#define VERSION 0" > src/config.h
	$(CC) -s -march=24kc -ffreestanding src/spi-config.c src/spi-tools.c -I src -Wall -Os -o bin/spi-config

spi-pipe:
	mkdir -p bin
	echo "#define VERSION 0" > src/config.h
	$(CC) -s -march=24kc -ffreestanding src/spi-pipe.c src/spi-tools.c -I src -Wall -Os -o bin/spi-pipe

all: spi-config spi-pipe

clean:
	rm -r bin

This should make it a bit more straightforward to experiment with the compiler options, if needed.

I believe the line
$ printf '\x01\x82\F3' | spi-pipe -d /dev/spidev0.0 | hexdump -C
should be
$ printf '\x01\x82\xF3' | spi-pipe -d /dev/spidev0.0 | hexdump -C

Just missing the 'x' for the last hex character.

J'ai en prévision d'intégrer l'outils à Yocto (déjà opérationnel en local) sur la meta-raspberry, c'est ok pour toi ?

Hi!

Appreciate the project a lot.

I'm trying to reverse engineer an spi driver and it would be super helpful to have some sort of "monitor the spi bytes being sent/recieved on a bus".

Let me know if there's any interest in this or if there are other projects better suited for this that you know of.

Thanks!

Hello,

Could you release/tag the latest version from master as v1.0.2 so that we can use the latest LSB_FIRST fixes more easily?

Thanks :)

Hi cpb-,

I have recently included a recipe for spi-tools in Buildroot:
https://patchwork.ozlabs.org/patch/547120/

We are currently fetching from the commit sha1 which currently points to the tip of the master branch, as there is no tagged version with autotools support.

I have submitted today two pull requests in order to fix two issues found with spi-tools since its inclusion in Buildroot. One is related to the build problem with musl as seen in:
http://autobuild.buildroot.net/results/ed8/ed89fae4a722b5d2adff9826bb88e55e58af77f6/
The other is related to the GIT_VERSION variable as can be seen in the patch mentioned in the beginning.

I'm opening this issue in order to suggest that a new version gets tagged in case you agree with the pull requests. I see that the autotools scripts already point to a 0.8.1 version but there is no tag to this version yet.
If you are able to generate a tag to this new version with autotools support and these fixes, I can then send a version update to Buildroot to this new tagged version, which will fix all of the mentioned issues without "out-of-tree" patches/hacks, and also setting its version to something more pretty than a commit sha1.

Thanks for spi-tools and for your attention.

Erico

Hi,
There is a bug in reading the last block of data ... if I set block size = 8 and then I have a data of not being multiple block size e.g. 27bytes then making it into chunks of 8+8+8+3=27 bytes, the last block which is 3bytes will not be received
I reviewed the code and made a patch to fix it

diff --git a/src/spi-pipe.c b/src/spi-pipe.c
index a619e72..67d33ae 100644
--- a/src/spi-pipe.c
+++ b/src/spi-pipe.c
@@ -162,14 +162,15 @@ int main (int argc, char * argv[])
                        if (nb <= 0)
                                break;
                }
-               if (nb <= 0)
+               if ((nb <= 0) && (offset == 0))
                        break;
 
+               transfer.len = offset;
                if (ioctl(fd, SPI_IOC_MESSAGE(1), & transfer) < 0) {
                        perror("SPI_IOC_MESSAGE");
                        break;
                }
-               if (write(STDOUT_FILENO, rx_buffer, blocksize) <= 0)
+               if (write(STDOUT_FILENO, rx_buffer, offset) <= 0)
                        break;
                if (blocknumber > 0)
                        blocknumber --;