[nuttx] STM32 SPI CRC

[[e-mail] [nuttx]]

Greg,

I'm working on a STM32 F3 device and accessing some slave devices that require / expect CRC enabled. Not all slave devices do CRC. Multiple devices can obviously exist on a SPI bus (with a sufficient number of chip selects of course) and not all of those devices may require / support CRC. So CRC needs to be configurable for each transfer / exchange.


Looking at the stm32/stm32_spi.c I see that while the polynomial is configured in spi_portinitialize() the driver does not appear to support CRC in general. I don't see the point of configuring the polynomial if CRC isn't supported, particularly because the value specified (7) is the power up default. But that's another issue.


Normally I'd implement this as a bitmask / flag I pass in a message structure but per the SPI interface (include/nuttx/spi/spi.h) requirese individual parameters (txbuffer, rxbuffer, nbytes) rather than a structure in spi_exchange(). So if I add a flag OR a new structure to replace the individual parameters, I break the system's SPI interface.


I am still researching some statements in the reference manual that seem to indicate hardware SPI CRC calculations are only supported for transfers of 1 or 2 bytes, and if that's true, I'll have to do it in higher level software as I'm sending larger buffers. BUT...assuming the CRC function of the peripheral isn't so stupidly limited and actually calculates it at completion of the DMA or something convenient like that, then I would like to add the capability to the driver and submit the changes to the project.



Before I can do that, however, I need to know how you would prefer to address the interface issue. I would prefer to model SPI after I2C and pass a "message" type structure which could contain:


txbuffer
rxbuffer
nwords
flags


and I would naturally set flags to:


SPI_M_ENABLECRC


to enable CRC on that particular exchange.


-Doug

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
I'm working on a STM32 F3 device and accessing some slave devices that require / expect CRC enabled. Not all slave devices do CRC. Multiple devices can obviously exist on a SPI bus (with a sufficient number of chip selects of course) and not all of those devices may require / support CRC. So CRC needs to be configurable for each transfer / exchange.
Looking at the stm32/stm32_spi.c I see that while the polynomial is configured in spi_portinitialize() the driver does not appear to support CRC in general. I don't see the point of configuring the polynomial if CRC isn't supported, particularly because the value specified (7) is the power up default. But that's another issue.

Is this an STM32-specific solution for automatic CRC generation? Any SPI drivers in NuttX must be platform specific and cannot depend specific features of a single MCU -- at least not without a lot of conditioning.

Post by ***@yahoo.com [nuttx]
Normally I'd implement this as a bitmask / flag I pass in a message structure but per the SPI interface (include/nuttx/spi/spi.h) requirese individual parameters (txbuffer, rxbuffer, nbytes) rather than a structure in spi_exchange(). So if I add a flag OR a new structure to replace the individual parameters, I break the system's SPI interface.
I am still researching some statements in the reference manual that seem to indicate hardware SPI CRC calculations are only supported for transfers of 1 or 2 bytes, and if that's true, I'll have to do it in higher level software as I'm sending larger buffers. BUT...assuming the CRC function of the peripheral isn't so stupidly limited and actually calculates it at completion of the DMA or something convenient like that, then I would like to add the capability to the driver and submit the changes to the project.

If CRC generation is a an STM32-specific feature, I would prefer a software based solution.

Post by ***@yahoo.com [nuttx]
txbuffer
rxbuffer
nwords
flags
SPI_M_ENABLECRC

Only if you implement that feature in all SPI drivers. I don't want any STM32-specific features built into common interfaces an a generic operating system.

Obviously this is something that I need to learn more about, but I will not break the common SPI interface to support an STM32-only feature. So, at least until I learn more, I am opposed to any changes to the SPI interface.

Greg

[[e-mail] [nuttx]]

I still don't like this change, but one additional though occurs to me...

Post by ***@yahoo.com [nuttx]
txbuffer
rxbuffer
nwords
flags
SPI_M_ENABLECRC

Let's not think about adding any paramters. The SPI interface is different from the I2C interface because it has a lock() method. So the typical use of SPI is:

- lock the interface
- Configure frequency and number of bits
- Perform the transfer, and
- unlock the interface

This locking guarantees that all of the interface parameters are correct and eliminates the multi-threading issues the I2C has (see the bug in the top-level TODO list "I2C NOT THREAD SAFE" around line 1608).

So, in SPI, you do not have to pack options into flags and you do not have to add any parameters to inter interfaces. You could add a new interface like SPI_CRCGENERATION and then it would work like this:

- lock the interface
- Configure frequency and number of bits AND the CRC generation options
- Perform the transfer, and
- unlock the interface

Now, what do we do if the hardware does not support automatic CRC generation? In that case, any call to SPI_CRCGENERATION should cause a compile time error and the driver must have a fallback to software CRC generation.

That is a much less invasive interface change.

Greg

[[e-mail] [nuttx]]

As near as I can tell by some quick, superficial research, SPI does not generally support a CRC. But there is SMBus and some I2C peripherals will support SMBus mode. From what I gather, SMBus mode operation is similar to I2C operation with the following exceptions:

- Only 7-bit addressing can be used.
- The SMBus standard describes a set of timeout values to ensure progress and throughput on the bus.
- Transmissions can optionally include a CRC byte, called Packet Error Check (PEC).
- Some hardware has PEC discrete input reports if the CRC was correct (because they are enable to perform the CRC verfication in time to NAK)
- Another discreate input, SMBALERT, allows a slave to get a master attention.
- A set of addresses has been reserved for protocol handling.

I am looking at the Atmel SAVM71 data sheet and it does support CRCs on its TWI peripheral, but only if it is configured in SMBus mode. At least, it only discusses CRC with regard to SMBus mode. Freescale Kinetis K60 is the same, the I2C peripheral supports an SMBus mode and CRC generation is only described with regard to SMBus mode.

The STM32 F4 data sheet, on the other hand, does not mention SMBus at all, but does support CRC's on its SPI peripheral.

- All SMBus devices support CRC, but
- I don't see any references to I2C devices supporting CRC but this might a an interface option in some devices.
- I suspect that the STM32 implementation is just to provide some partial support for SMBus.

Are you certain the the devices that you need CRC support are not SMBus devices?

If this understanding is correct, then my position would be this:

- I do not want to extend the I2C interface to support SMBus.
- Rather, I would want to create a new SMBus interface that supports the unique features of SMBus.

Do I have that right? If so, let's not mix protocols even if they are very similar (like STM32 seems to have done).

Greg

[[e-mail] [nuttx]]

As near as I can tell by some quick, superficial research, SPI does not generally support a CRC. But there is SMBus and some I2C peripherals will support SMBus mode.

Hmm.. I am not sure how I segued from SPI to I2C. Sorry. Happens to me often. So just ignore the previous post, please.

Just as an update: There is no refrence to SPI CRC generation support in the user manual for Atmel, Freescale, or NXP parts. So this appears to be a feature that is unique to the STM32.

Since it is not a feature that is support by any MCU other than STM32, I would prefer such an MCU-specific feature not be exposed at all in any changes to the common SPI interface. A software CRC solution was be much preferred.

I would, in fact, insist on a software CRC solution on any common drivers that that you might find under nutt/drivers. You private drivers may exploit features unique to the STM32 part, but no common drivers may do that.

I am not completely opposed to my counter-proposal of https://groups.yahoo.com/neo/groups/nuttx/conversations/messages/10881, but I still would not permit the auto-CRC generation logic in any common drivers. Such drivers could use the auto-CRC generation logic, but would also need to support a fall-back software CRC generation in order to be portable.

Greg

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
If CRC generation is a an STM32-specific feature, I would prefer a software based solution.

I don't know for sure, but I can't imagine it being STM32-specific since the concept is well known and supported on several slave devices I looked at earlier in my design. I did wind up settling on an ST slave part and that too supports CRC by default, which explains why I want CRC enabled and would preferably like the hardware to take care of it.


With regard to the F3's implementation my confusion was over the statement:


------
Two separate CRC calculators are implemented in order to check the reliability of
transmitted and received data. The SPI offers CRC8 or CRC16 calculation independently of
the frame data length, which can be fixed to 8-bit or 16-bit. For all the other data frame
lengths, no CRC is available.
------



To me a frame is an entire packet or transfer that consists of N words, but after reading more thoroughly, ST defines a frame as a word. So by their definition the F3 definitely supports automatic CRC generation and validation for transfers of N words so long as the word size is 8 or 16 bits.


Adding a SPI_CRCGENERATION method to the interface would work, and in addition to not breaking the existing interface I imagine it would be easier and cleaner to include or exclude that method at compile time based on the platform configuration. If there is no hardware support on the platform and SPI_CRCGENERATION is enabled then I imagine the fallback would mean doing the CRC generation and validation in spi_exchange() but then we're talking memory allocation or copying in the driver unless the user pre-allocates the extra byte(s) for CRC.



FYI, the F3 requires that the CRC feature be enabled or disabled before the peripheral is enabled and the transfer commences, i.e. this would best be done in spi_portinitialize() along with all the other tweaking of SPI_CR1.

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
Adding a SPI_CRCGENERATION method to the interface would work, and in addition to not breaking the existing interface I imagine it would be easier and cleaner to include or exclude that method at compile time based on the platform configuration. If there is no hardware support on the platform and SPI_CRCGENERATION is enabled then I imagine the fallback would mean doing the CRC generation and validation in spi_exchange() but then we're talking memory allocation or copying in the driver unless the user pre-allocates the extra byte(s) for CRC.

No, that would be unacceptable. There will be no modification to the existing spi_exchange() method implementations of any other MCUs. That is certain.

The upper driver that uses SPI will be required to implement the software CRC as needed by the device, not every lower SPI driver. Use of SPI_CRCGENERATION would be an option for improved performance with the STM32 only.

Upper drivers that need to use SPI CRC should do something like:

#ifdef CONFIG_SPI_HARDWARECRC
SPI_CRCGENERATION(dev, true);
#else
software_crc(buffer, buflen);
#endif

They must *always* profive the implementation of a software CRC for portability.

The definition in in include/nuttx/spi/spi.h would have to be something like:

#ifdef CONFIG_SPI_HARDWARECRC
# define SPI_CRCGENERATION(d,e) ((d)->crcgeneration(d,e))
#else
# define SPI_CRCGENERATION(d,e) ASSERT((e) == false)
#endif

No other option will be considered involving modification to lower SPI drivers. CRC is part of the protocol implemented in the upper driver that uses the SPI device and, other than the special case of the STM32, not the responsibility of the lower SPI drivers.

Post by ***@yahoo.com [nuttx]
FYI, the F3 requires that the CRC feature be enabled or disabled before the peripheral is enabled and the transfer commences, i.e. this would best be done in spi_portinitialize() along with all the other tweaking of SPI_CR1.

Then would it be useful for supporting multiple SPI devices on the same bus, some with CRC and some without? I suppose that you would have to reinitialize the peripheral on each change in CRC state then.

I personally have no concern or interest in the F3 SPI implementation. But I am *very* concerned about the portability and integrity of the common SPI interfaces. I will no nothing to degrade that. There will be no tail wagging the dog.

Greg

[[e-mail] [nuttx]]

Notice that the CRC8 algorithm is already available in the source tree: https://bitbucket.org/patacongo/nuttx/src/master/libc/misc/lib_crc8.c

So any software CRC solution would be a trivial exercise (provided that the CRC algorithms are compatible).

Currently there are no devices using SPI CRC and no MCU SPI peripheral that supports H/W assisted SPI CRCs that I know of other than STM32.

The STM32-specific hardware assisted SPI CRC generation, brings up several system level issues. I am uncompromising when it comes to issues of the integrity of common OS interfaces.

Greg

[[e-mail] [nuttx]]

Yes, I am familiar with that and in fact had planned to use it before I discovered the STM32 did it in hardware. As much as I've made a living writing software, I like to avoid duplicating functionality provided by the hardware.

Post by ***@yahoo.com [nuttx]
Then would it be useful for supporting multiple SPI devices on the same bus,
some with CRC and some without? I suppose that you would have to reinitialize
the peripheral on each change in CRC state then.

If there are multiple devices on the bus, some supporting CRC and some not, then yes, the peripheral would have to be disabled and reinitialized with CRCEN (among other things) since at least on the STM32 toggling the PE (peripheral enable) bit reverts most if not all settings back to defaults. This effectively requires calling spi_portinitialize() again.

Post by ***@yahoo.com [nuttx]
The upper driver that uses SPI will be required to implement the software CRC
as needed by the device, not every lower SPI driver. Use of
SPI_CRCGENERATION would be an option for improved performance
with the STM32 only.

To clarify -- it appears that you:


1) Are opposed to any modification to the lower half drivers other than those that provide hardware CRC support like STM32.


2) Are not opposed to adding the SPI_CRCGENERATION method to the lower-half SPI interface so upper half drivers can enable it if available on the platform (STM32, for example).


Correct?

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
1) Are opposed to any modification to the lower half drivers other than those that provide hardware CRC support like STM32.

Yes, that would be a big, big pointless job. Who would implement all of those changes? How is going to test the changes all of the modified drivers. There is no need and no benefit for that kind of risk and chaos.


The are not broken and don't need this kind of fixing.

Post by ***@yahoo.com [nuttx]
2) Are not opposed to adding the SPI_CRCGENERATION method to the lower-half SPI interface so upper half drivers can enable it if available on the platform (STM32, for example).

Perhaps SPI_HWFEATURES would be a better interface, accepting a set of flags where CRCGENERATION is one hardware feature that could be enabled. That would simplify this the discussion in the future if there is similar issue with other SPI hardware-specific features.


And to this I would add:


3) Upper drivers the use SPI with CRC should not be dependent upon the HW feature. They should be capable of generating CRC8 in software if not supported by HW accelerators.


Greg

[[e-mail] [nuttx]]

I will implement the changes to the Kconfig files and include/nuttx/spi/spi.h. I will try to get that done later today.


Greg

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
Perhaps SPI_HWFEATURES would be a better interface, accepting a
set of flags where CRCGENERATION is one hardware feature that could
be enabled. That would simplify this the discussion in the future if there is
similar issue with other SPI hardware-specific features.

That sounds good to me. It's basically what I was looking for in the first place (a flags variable) and this doesn't require any changes to existing code.

[[e-mail] [nuttx]]

Here is the interface definition:

https://bitbucket.org/patacongo/nuttx/commits/85331739bfeca9e773a75c45cb8e0d22e785fca8
https://bitbucket.org/nuttx/arch/commits/10e420a652fd8bcf9441f542172182f5db05e70a

To use it, you would add this to the STM32 Kconfig file when the SPI driver is selected. You may make that conditional to save space on STM32 platforms that don't need the feature:

select SPI_CRCGENERTAION

Greg

[[e-mail] [nuttx]]

There is one logic flaw in the interface that occurs to me.


If one driver calls:


SPI_HWFEATURES(dev,HWFEAT_CRCGENERATION);


Then the next driver runs and does not call SPI_HWFEATURES, then the previous value will be retained and the second use of the SPI interface will get CRC generation enabled. The design needs to be changed to allow legacy SPI drivers to work as they do now without modification when intermingled with newer drivers that call SPI_HWFEATURES().


Greg

[[e-mail] [nuttx]]

This should take care of that problem:

https://bitbucket.org/patacongo/nuttx/commits/03a631926e6b9ae371f635245c11f424fbf12b75
https://bitbucket.org/nuttx/arch/commits/95cea5cd330f0878627cdd73cc113756b84f28b0

Greg

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
SPI_HWFEATURES(dev,HWFEAT_CRCGENERATION);
Then the next driver runs and does not call SPI_HWFEATURES, then the
previous value will be retained and the second use of the SPI interface will
get CRC generation enabled. The design needs to be changed to allow
legacy SPI drivers to work as they do now without modification when
intermingled with newer drivers that call SPI_HWFEATURES().

You have a better understanding of the high level picture and use cases than I do, obviously, but if I understand the lower-halves correctly it seems that each connection to the driver implies an initialization process (spi_portinitialize).


I can't speak to other platforms or devices, but on the STM32 F3 each connection to the driver toggles the peripheral enable, and that restores default settings which are then tweaked as needed. On the STM32 F3, incidentally, CRC is disabled by default so even if a prior instantiation of the software enabled it, it would be disabled at that point.


Do we still have a problem?


Another thing that was rattling around in my brain: assuming you're still going with the idea of a common driver interface method that passes only a flag value, I'm thinking passing a structure that contained the flag would be better as it would allow us to easily add structure members to contain data that must be sent to the hardware to enable a given feature.


Some examples:


1) In the case of CRC perhaps we might want to give the upper half the ability to change the polynomial. So the upper half would set the "HARDWARECRC" flag and fill in the "poly" structure member.


2) Say some future SPI hardware supports a kind of scrambling feature so data sent over the wire is encrypted or otherwise obfuscated. And assume that this feature utilizes a random number generator, the seed for which must be set when the scrambling is enabled. To enable the scrambling the user would set the flag and pass the seed value down in a structure member.


See what I mean? Passing a simple flag solves my problem today so I'm happy, but it won't necessarily be ideal in the long term. Something to think about.

[[e-mail] [nuttx]]

Post by ***@yahoo.com [nuttx]
You have a better understanding of the high level picture and use cases than I do, obviously, but if I understand the lower-halves correctly it seems that each connection to the driver implies an initialization process (spi_portinitialize).

That is not right in the sense that there are no connections. For all architectures, up_spiinitialize() is meant to be called only once per port. Most have protection to assure that they do nothing if they that function is called more than once. That is a bug in the STM32 version, but hopefully benign. It is not a behavior that you should consider normal.

Normal is the that up_spinitialize() action occurs only once per SPI peripheral when it is first initialized.

Post by ***@yahoo.com [nuttx]
I can't speak to other platforms or devices, but on the STM32 F3 each connection to the driver toggles the peripheral enable, and that restores default settings which are then tweaked as needed. On the STM32 F3, incidentally, CRC is disabled by default so even if a prior instantiation of the software enabled it, it would be disabled at that point.

There are no pre-driver connects. There is a single state structure for each SPI peripheral, not one per driver. If one driver sets the frequency to a particular driver, then it effects the other drivers too, unless they lock the device and reset the frequency.

Same will be true of the hwfeatures. When one driver selects the hwfeature, it will affect all all other drivers using the SPI interface unless they remember to reset the hwfeatures just as they now have to reset the number of bits, SPI mode, and frequency.

Post by ***@yahoo.com [nuttx]
Do we still have a problem?

Conceptually, yes. However, I added calls to SPI_HWFEATURES(spi, 0); to *EVERY* spi device driver.

Post by ***@yahoo.com [nuttx]
Another thing that was rattling around in my brain: assuming you're still going with the idea of a common driver interface method that passes only a flag value, I'm thinking passing a structure that contained the flag would be better as it would allow us to easily add structure members to contain data that must be sent to the hardware to enable a given feature.

Too late. The interface changes are already done and there will be no more changes in the near future. Please live with what I have given you.

You could, in fact, replace all of SPI_SETMODE, SPI_SETBITS, SPI_HWFEATURES, and SPI_SETFEQUENCY with a single call, say SPI_SETCONFIGURATION. But that will not e happening in the foreseeable future.

Post by ***@yahoo.com [nuttx]
..
See what I mean? Passing a simple flag solves my problem today so I'm happy, but it won't necessarily be ideal in the long term. Something to think about.

Something to think about some other day. Let's not over design an interface that have no clients for now.

Greg

[[e-mail] [nuttx]]

Hi guys,
I'm having the same problem, needing hardware CRC support for the STM32 SPI.

Is there any progress on this, or can I help with this somehow?

Regards Pascal Speck

[[e-mail] [nuttx]]

The interface hooks were added. See line 168 of include/nuttx/spi/spi.h. But there is no implementation of the CRC generation code for any MCU.


Greg

[[e-mail] [nuttx]]

The SPI slave interface is defined in include/nuttx/spi/slave.h. There an implementation for the SAMV71 at arch/arm/src/samv7/sam_spi_slave.c. Nothing has been done by anyone for STM32.


There are serious performance issues with implementing SPI slave in software. Read the issues listed in include/nuttx/spi/slave.h. It is VERY slow. If you want proper SPI slave performance, then you might consider:


1. Don't use a generic SPI slave interface such as arch/arm/src/samv7/sam_spi_slave.c. Instead build the slave support into your own driver so that you can implement things in a more integrated, more efficient way.


2. Better, use an FPGA.


Greg

[Sebastien Lorquet [e-mail] [nuttx]]

I think STM32 supports DMA for SPI slave, or am I wrong?

Sebastien

Post by ***@yahoo.com [nuttx]
The SPI slave interface is defined in include/nuttx/spi/slave.h. There an
implementation for the SAMV71 at arch/arm/src/samv7/sam_spi_slave.c. Nothing
has been done by anyone for STM32.
There are serious performance issues with implementing SPI slave in software.
Read the issues listed in include/nuttx/spi/slave.h. It is VERY slow. If you
1. Don't use a generic SPI slave interface such as
arch/arm/src/samv7/sam_spi_slave.c. Instead build the slave support into your
own driver so that you can implement things in a more integrated, more efficient
way.
2. Better, use an FPGA.
Greg

[[e-mail] [nuttx]]

The SAMV71 supports SPI DMA as well. The issue is that there is no design in place to use the DMA:

https://bitbucket.org/nuttx/nuttx/src/e5388ad12713ba1804bece7834062ba686659365/include/nuttx/spi/slave.h?at=master&fileviewer=file-view-default#slave.h-394

Lots of issues. Basically, you can't know the size of the DMA in advance, so you would have to do something really clever. Also the most time critical parts of the processing are initially when you receive some kind of command and have to set up the DMA for the response. DMA is only useful for streaming data out or out of a buffer in memory. It doesn't help with protocol issues.

Most of this protocol issues can be resolve by NOT using a generic SPI slave driver, but building the SPI slave logic into your device driver. That is because the device driver understands the protocol. A generic SPI slave layer does not.

Greg

[Pascal Speck [e-mail] [nuttx]]

I'm just wondering (especially for stm32) how I'll find a clean way to use the normal stm32_spi.c driver but changing some registers during initialization.

( basically clear the MSTR bit and enable CRC "before" SPI is enabled )

To prevent code duplication I would prefer a way that is accepted by nuttx community so that the stm32_spi.c supports setting project specific options during initialization.
Is there any preferred interface for doing this?

Regards Pascal

Post by ***@yahoo.com [nuttx]
https://bitbucket.org/nuttx/nuttx/src/e5388ad12713ba1804bece7834062ba686659365/include/nuttx/spi/slave.h?at=master&fileviewer=file-view-default#slave.h-394
Lots of issues. Basically, you can't know the size of the DMA in advance, so you would have to do something really clever. Also the most time critical parts of the processing are initially when you
receive some kind of command and have to set up the DMA for the response. DMA is only useful for streaming data out or out of a buffer in memory. It doesn't help with protocol issues.
Most of this protocol issues can be resolve by NOT using a generic SPI slave driver, but building the SPI slave logic into your device driver. That is because the device driver understands the
protocol. A generic SPI slave layer does not.
Greg

[[e-mail] [nuttx]]

I doubt that that would work. The SPI master driver is completely different in its operation than an SPI master driver. I would be surprised if there is any code re-use.

[Pascal Speck [e-mail] [nuttx]]

Thanks Greg for your very fast answer,
I'll have a look at the current sources you mentioned and try to implement it in a way that it is suitable for me..
I just don't wanted to do twice the work.
In my spi-slave setup it might be suitable to setup the dma before i get the select from the master, because I'll use it in a kind of half-duplex implementation, so I don't see any performance
trade-offs for now..
Regards Pascal

Post by Sebastien Lorquet ***@lorquet.fr [nuttx]
I think STM32 supports DMA for SPI slave, or am I wrong?
Sebastien

The SPI slave interface is defined in include/nuttx/spi/slave.h. There an
implementation for the SAMV71 at arch/arm/src/samv7/sam_spi_slave.c. Nothing
has been done by anyone for STM32.
There are serious performance issues with implementing SPI slave in software.
Read the issues listed in include/nuttx/spi/slave.h. It is VERY slow. If you
1. Don't use a generic SPI slave interface such as
arch/arm/src/samv7/sam_spi_slave.c. Instead build the slave support into your
own driver so that you can implement things in a more integrated, more efficient
way.
2. Better, use an FPGA.
Greg