spi: bitbang fixes and passive serial driver
cover spi: bitbang fixes and passive serial driver
This series includes a bugfix and adds LSB mode for the bitbang driver. It also includes a new character device driver to passively clock a firmware into an FPGA.
It is an resend after the cc to linux-arm-kernel list got fixed.
Michael Grzeschik (3):
spi: bitbang: fix shift for getmosi
spi: bitbang: add lsb first support
spi: psdev: add passive serial driver
drivers/spi/Kconfig | 9 + drivers/spi/Makefile | 1 + drivers/spi/spi-bitbang-txrx.h | 98 +++++--- drivers/spi/spi-bitbang.c | 3 +- drivers/spi/spipsdev.c | 520 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 601 insertions(+), 30 deletions(-) create mode 100644 drivers/spi/spipsdev.c
Signature · 1 line
1.9.0
PATCH 1/3 spi: bitbang: fix shift for getmosi
The driver needs to shift the word bit after reading the mosi bit. Otherwise the return word will have an Off-by-one bit value.
--- drivers/spi/spi-bitbang-txrx.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/spi/spi-bitbang-txrx.h +2 -2
@@ -61,10 +61,10 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,spidelay (nsecs);/* sample MSB (from slave) on leading edge */ - word <<=1 ;if ((flags &SPI_MASTER_NO_RX ) ==0 ) word |=getmiso (spi);setsck (spi, cpol); + word <<=1 ; }return word; }
@@ -89,9 +89,9 @@ bitbang_txrx_be_cpha1(struct spi_device *spi,spidelay (nsecs);/* sample MSB (from slave) on trailing edge */ - word <<=1 ;if ((flags &SPI_MASTER_NO_RX ) ==0 ) word |=getmiso (spi); + word <<=1 ; }return word; }
Signature · 1 line
1.9.0
Mark Brown
maintainerPATCH 1/3 spi: bitbang: fix shift for getmosi
Quoted 2 lines · Michael Grzeschik
The driver needs to shift the word bit after reading the mosi bit. Otherwise the return word will have an Off-by-one bit value.
This isn't exactly new code... do we understand why nobody has noticed this before?
drivers/spi/spi-bitbang-txrx.h bitbang_txrx_be_cpha0(struct spi_device *spi, · line 68
Quoted 10 lines
drivers/spi/spi-bitbang-txrx.h +1
-1@@ -61,10 +61,10 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,spidelay (nsecs);/* sample MSB (from slave) on leading edge */ - word <<=1 ;if ((flags &SPI_MASTER_NO_RX ) ==0 ) word |=getmiso (spi);setsck (spi, cpol); + word <<=1 ; }
Just looking at the context here it's not obvious to me that this is
helping - it means that the last bit we read is going to be shifted
which seems wrong, we ought to be reading into LSB.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140312/7bbc3eaf/attachment.sig>
PATCH 1/3 spi: bitbang: fix shift for getmosi
Quoted 7 lines · Mark Brown
Quoted 2 lines · Michael Grzeschik
The driver needs to shift the word bit after reading the mosi bit. Otherwise the return word will have an Off-by-one bit value.
This isn't exactly new code... do we understand why nobody has noticed this before?
I too suspect that the bug (if there is any) is elsewhere.
Quoted 14 lines
drivers/spi/spi-bitbang-txrx.h bitbang_txrx_be_cpha0(struct spi_device *spi, · line 68
Quoted 10 lines
drivers/spi/spi-bitbang-txrx.h +1
-1@@ -61,10 +61,10 @@ bitbang_txrx_be_cpha0(struct spi_device *spi,spidelay (nsecs);/* sample MSB (from slave) on leading edge */ - word <<=1 ;if ((flags &SPI_MASTER_NO_RX ) ==0 ) word |=getmiso (spi);setsck (spi, cpol); + word <<=1 ; }Just looking at the context here it's not obvious to me that this is helping - it means that the last bit we read is going to be shifted which seems wrong, we ought to be reading into LSB.
It might be more robust to
if (!(flags & SPI_MASTER_NO_RX) && getmiso(spi))
word |= 1;instead. This decouples the construction of the received bits buffer from whatever the getmiso() implementation might look like. That's just a thought after the recent GPIO discussion about whether 1/0 is given or should not be assumed (and I still suspect that "!!x" need not result in exactly 1/0 values).
And I agree with Mark that the "late shift" is most probably wrong. Just noticed the resend too late and responded in the other thread.
virtually yours
Gerhard Sittig
Signature · 3 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
PATCH 2/3 spi: bitbang: add lsb first support
In reply to Michael GrzeschikThe bitbang spi driver currently only supports the MSB mode. This patch adds the possibility to clock the data in LSB mode.
--- drivers/spi/spi-bitbang-txrx.h | 98 +++++++++++++++++++++++++++++------------- drivers/spi/spi-bitbang.c | 3 +- 2 files changed, 71 insertions(+), 30 deletions(-)
drivers/spi/spi-bitbang-txrx.h +69 -29
@@ -49,22 +49,42 @@ bitbang_txrx_be_cpha0(struct spi_device *spi, {/* if (cpol == 0) this is SPI_MODE_0; else this is SPI_MODE_2 */ -/* clock starts at inactive polarity */ -for (word <<= (32 - bits);likely (bits); bits--) { - -/* setup MSB (to slave) on trailing edge */ -if ((flags &SPI_MASTER_NO_TX ) ==0 ) -setmosi (spi, word & (1 <<31 )); -spidelay (nsecs);/* T(setup) */ - -setsck (spi, !cpol); -spidelay (nsecs); - -/* sample MSB (from slave) on leading edge */ -if ((flags &SPI_MASTER_NO_RX ) ==0 ) - word |=getmiso (spi); -setsck (spi, cpol); - word <<=1 ; +if (spi->mode &SPI_LSB_FIRST ) { +/* clock starts at inactive polarity */ +for (;likely (bits); bits--) { + +/* setup MSB (to slave) on trailing edge */ +if ((flags &SPI_MASTER_NO_TX ) ==0 ) +setmosi (spi, word &1 ); +spidelay (nsecs);/* T(setup) */ + +setsck (spi, !cpol); +spidelay (nsecs); + +/* sample LSB (from slave) on leading edge */ +if ((flags &SPI_MASTER_NO_RX ) ==0 ) + word |=getmiso (spi); +setsck (spi, cpol); + word >>=1 ; + } + }else { +/* clock starts at inactive polarity */ +for (word <<= (32 - bits);likely (bits); bits--) { + +/* setup MSB (to slave) on trailing edge */ +if ((flags &SPI_MASTER_NO_TX ) ==0 ) +setmosi (spi, word & (1 <<31 )); +spidelay (nsecs);/* T(setup) */ + +setsck (spi, !cpol); +spidelay (nsecs); + +/* sample MSB (from slave) on leading edge */ +if ((flags &SPI_MASTER_NO_RX ) ==0 ) + word |=getmiso (spi); +setsck (spi, cpol); + word <<=1 ; + } }return word; }
@@ -76,22 +96,42 @@ bitbang_txrx_be_cpha1(struct spi_device *spi, {/* if (cpol == 0) this is SPI_MODE_1; else this is SPI_MODE_3 */ -/* clock starts@inactive polarity */ -for (word <<= (32 - bits);likely (bits); bits--) { +if (spi->mode &SPI_LSB_FIRST ) { +/* clock starts at inactive polarity */ +for (;likely (bits); bits--) { + +/* setup MSB (to slave) on leading edge */ +setsck (spi, !cpol); +if ((flags &SPI_MASTER_NO_TX ) ==0 ) +setmosi (spi, word &1 ); +spidelay (nsecs);/* T(setup) */ + +setsck (spi, cpol); +spidelay (nsecs); + +/* sample MSB (from slave) on trailing edge */ +if ((flags &SPI_MASTER_NO_RX ) ==0 ) + word |=getmiso (spi); + word >>=1 ; + } + }else { +/* clock starts at inactive polarity */ +for (word <<= (32 - bits);likely (bits); bits--) { -/* setup MSB (to slave) on leading edge */ -setsck (spi, !cpol); -if ((flags &SPI_MASTER_NO_TX ) ==0 ) -setmosi (spi, word & (1 <<31 )); -spidelay (nsecs);/* T(setup) */ +/* setup MSB (to slave) on leading edge */ +setsck (spi, !cpol); +if ((flags &SPI_MASTER_NO_TX ) ==0 ) +setmosi (spi, word & (1 <<31 )); +spidelay (nsecs);/* T(setup) */ -setsck (spi, cpol); -spidelay (nsecs); +setsck (spi, cpol); +spidelay (nsecs); -/* sample MSB (from slave) on trailing edge */ -if ((flags &SPI_MASTER_NO_RX ) ==0 ) - word |=getmiso (spi); - word <<=1 ; +/* sample MSB (from slave) on trailing edge */ +if ((flags &SPI_MASTER_NO_RX ) ==0 ) + word |=getmiso (spi); + word <<=1 ; + } }return word; }
drivers/spi/spi-bitbang.c +2 -1
@@ -432,7 +432,8 @@ int spi_bitbang_start(struct spi_bitbang *bitbang)spin_lock_init (&bitbang->lock);if (!master->mode_bits) - master->mode_bits =SPI_CPOL |SPI_CPHA | bitbang->flags; + master->mode_bits =SPI_CPOL |SPI_CPHA |SPI_LSB_FIRST + | bitbang->flags;if (master->transfer || master->transfer_one_message)return -EINVAL ;
Signature · 1 line
1.9.0
PATCH 3/3 spi: psdev: add passive serial driver
In reply to Michael GrzeschikThis patch introduces the psdev driver. It is used to communicate to an Altera passive serial connected FPGA by writing into an character device.
It simply initates the protocol on open and clocks the data into the FPGA by using the underlying SPI bus on write. The used SPI bus can also be an bitbang SPI bus. On close it will check for the response of the FPGA and tells if the write process was successful.
--- drivers/spi/Kconfig | 9 + drivers/spi/Makefile | 1 + drivers/spi/spipsdev.c | 520 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 530 insertions(+) create mode 100644 drivers/spi/spipsdev.c
drivers/spi/Kconfig +9 -0
@@ -208,6 +208,15 @@ config SPI_GPIO GPIO operations, you should be able to leverage that for better speed with a custom version of this driver; see the source code. +config SPI_PASSIVE_SERIAL + tristate "GPIO-based bitbanging SPI Altera Passive Serial" + depends on GPIOLIB + select SPI_BITBANG + help + This driver uses GPIOs to communicate by alteras passive serial + protocol. It creates an character device, that can be used to + simply cat RBF Firmware images into the FPGA. + config SPI_IMX tristate "Freescale i.MX SPI controllers" depends on ARCH_MXC || COMPILE_TEST
drivers/spi/Makefile +1 -0
@@ -8,6 +8,7 @@ ccflags-$(CONFIG_SPI_DEBUG) := -DDEBUG # config declarations into driver model code obj-$(CONFIG_SPI_MASTER) += spi.o obj-$(CONFIG_SPI_SPIDEV) += spidev.o +obj-$(CONFIG_SPI_PASSIVE_SERIAL) += spipsdev.o # SPI master controller drivers (bus) obj-$(CONFIG_SPI_ALTERA) += spi-altera.o
drivers/spi/spipsdev.c new file +520 -0
520 lines not on this page · raw message
Signature · 1 line
1.9.0
Mark Brown
maintainerPATCH 3/3 spi: psdev: add passive serial driver
Quoted 8 lines · Michael Grzeschik
This patch introduces the psdev driver. It is used to communicate to an Altera passive serial connected FPGA by writing into an character device.
It simply initates the protocol on open and clocks the data into the FPGA by using the underlying SPI bus on write. The used SPI bus can also be an bitbang SPI bus. On close it will check for the response of the FPGA and tells if the write process was successful.
Having read the above I'm still not entirely sure what this is supposed
to do. It sounds like it might be something to do with programming
FPGAs but that doesn't seem like something for the SPI subsystem, it
sounds like something for a FPGA handling subsystem. People keep
talking about that, there's a bunch of other stuff to integrate with
there like loading DTs for the downloeaded bitstream. I'm not sure what
the status is there.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140312/47cf4251/attachment.sig>
PATCH 3/3 spi: psdev: add passive serial driver
Quoted 11 lines · Mark Brown
Quoted 8 lines · Michael Grzeschik
This patch introduces the psdev driver. It is used to communicate to an Altera passive serial connected FPGA by writing into an character device.
It simply initates the protocol on open and clocks the data into the FPGA by using the underlying SPI bus on write. The used SPI bus can also be an bitbang SPI bus. On close it will check for the response of the FPGA and tells if the write process was successful.
Quoted 7 lines
Having read the above I'm still not entirely sure what this is supposed to do. It sounds like it might be something to do with programming FPGAs but that doesn't seem like something for the SPI subsystem, it sounds like something for a FPGA handling subsystem. People keep talking about that, there's a bunch of other stuff to integrate with there like loading DTs for the downloeaded bitstream. I'm not sure what the status is there.
Ok, so first of all it needs better documentation, also for the bindings. I will add some.
Second, the driver has "per se" nothing to do with SPI, beside it needs a clock and data line to transfer an image to the FPGA. (One way,
therefor: passive). It needs to use some SPI controller to do that. Even the SPI Bitbang driver. Because of this, it depends on the SPI framework.
Beside the SCLK and MOSI, the protocol needs to toggle and check some GPIOs for protocol start and end. For that I chose the posix open and close, so that in userland it can be used as simple as $(cat firmware.rbf > /dev/psdev0). This protocol also needs no SS line.
It is actually not more than that. I took the spidev character device driver as reference implementation. Any Idea where to put this driver instead.
Regards,
Michael
Signature · 4 lines
Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Mark Brown
maintainerPATCH 3/3 spi: psdev: add passive serial driver
Quoted 5 lines · Michael Grzeschik
Second, the driver has "per se" nothing to do with SPI, beside it needs a clock and data line to transfer an image to the FPGA. (One way,
therefor: passive). It needs to use some SPI controller to do that. Even the SPI Bitbang driver. Because of this, it depends on the SPI framework.
Lots of drivers use SPI but they sit in the relevant frameworks rather than in the SPI framework.
Quoted 3 lines
It is actually not more than that. I took the spidev character device driver as reference implementation. Any Idea where to put this driver instead.
Like I say somewhere where it sits with other FPGA programming stuff (if
that's what it does) would be my first thought.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140314/57c1f7d4/attachment-0001.sig>
PATCH 3/3 spi: psdev: add passive serial driver
Quoted 8 lines · Mark Brown
Quoted 3 lines · Michael Grzeschik
It is actually not more than that. I took the spidev character device driver as reference implementation. Any Idea where to put this driver instead.
Like I say somewhere where it sits with other FPGA programming stuff (if that's what it does) would be my first thought.
I'd suggest you try to use drivers/firmware.
That subsystem is a bit x86-centrix at the moment but looking at the stuff that is in there (like supporting BIOS updates through sysfs and other low-level firmware communication) this stuff is close enough.
Yours,
Linus Walleij
PATCH 3/3 spi: psdev: add passive serial driver
Quoted 7 lines · Linus Walleij
Quoted 6 lines · Mark Brown
Quoted 3 lines · Michael Grzeschik
It is actually not more than that. I took the spidev character device driver as reference implementation. Any Idea where to put this driver instead.
Quoted 8 lines
Quoted 2 lines
Like I say somewhere where it sits with other FPGA programming stuff (if that's what it does) would be my first thought.
I'd suggest you try to use
drivers/firmware.That subsystem is a bit x86-centrix at the moment but looking at the stuff that is in there (like supporting BIOS updates through sysfs and other low-level firmware communication) this stuff is close enough.
Indeed, the directory could need some refurbishing. It is also not even sourced by drivers/Kconfig but instead by the architectures that depend on it. Beside that, it seems to be the place to be.
Thanks,
Michael
Signature · 4 lines
Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Re: [PATCH 3/3] spi: psdev: add passive serial driver
In reply to Michael Grzeschik?????, 12 ????? 2014, 16:53 +01:00 ?? Michael Grzeschik <[email protected]>:
Quoted 8 lines
This patch introduces the psdev driver. It is used to communicate to an Altera passive serial connected FPGA by writing into an character device.
It simply initates the protocol on open and clocks the data into the FPGA by using the underlying SPI bus on write. The used SPI bus can also be an bitbang SPI bus. On close it will check for the response of the FPGA and tells if the write process was successful.
As a suggestion, I want to advise you to use GPIO functions based on descriptors (xx_gpiod_xx()) for the new drivers.
This will simplify code a bit, allows you to use the driver for non-DT boards without creating any new global headers for platform_data and eliminate the possible confusion with active levels for these GPIOs in the future.
---
PATCH 3/3 spi: psdev: add passive serial driver
In reply to Michael GrzeschikQuoted 16 lines · Michael Grzeschik
This patch introduces the psdev driver. It is used to communicate to an Altera passive serial connected FPGA by writing into an character device.
It simply initates the protocol on open and clocks the data into the FPGA by using the underlying SPI bus on write. The used SPI bus can also be an bitbang SPI bus. On close it will check for the response of the FPGA and tells if the write process was successful.
Signed-off-by Michael Grzeschik <[email protected]>--- drivers/spi/Kconfig | 9 + drivers/spi/Makefile | 1 + drivers/spi/spipsdev.c | 520 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 530 insertions(+) create mode 100644 drivers/spi/spipsdev.c
[...]
drivers/spi/spipsdev.c line 310
Quoted 4 lines
drivers/spi/spipsdev.c new file +4
-0+static const struct of_device_id psdev_dt_ids[] = { + { .compatible ="altr,passive-serial" }, + {}, +};
There should be a binding document for this as part of the series.
Thanks,
Mark.
PATCH 3/3 spi: psdev: add passive serial driver
In reply to Michael GrzeschikHi,
Quoted 9 lines · Michael Grzeschik
This patch introduces the psdev driver. It is used to communicate to an Altera passive serial connected FPGA by writing into an character device.
It simply initates the protocol on open and clocks the data into the FPGA by using the underlying SPI bus on write. The used SPI bus can also be an bitbang SPI bus. On close it will check for the response of the FPGA and tells if the write process was successful.
[...] +/*-------------------------------------------------------------------------*/
drivers/spi/spipsdev.c line 309
Quoted 5 lines
drivers/spi/spipsdev.c new file +5
-0+#ifdef CONFIG_OF + +static const struct of_device_id psdev_dt_ids[] = { + { .compatible ="altr,passive-serial" }, + {},
^
<nit>
Since '{}' always must be the last entry in the initializer list, the comma doesn't make sense here.
Omitting it would cause entries added after it (e.g. due to a bad merge conflict resolution) to generate a compile error.
</nit>
Lothar Wa?mann
Signature · 9 lines
___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________