Issue with SPI Chip Select

RogerClark
Tue Jun 09, 2015 10:32 am
This is a spin off thread from the issue with the SD lib

Basically the issue is that in SPI, that pin PA4 is technically the hardware Software Select (aka Chip Select etc) pin

However there seems to be a bug / feature in the F103 and F4 (and possibly other STM32 processors), where the hardware SPI Software Select pin, known as NSS basically doesn’t do anything.

Whats worse is that when the code sets up the SPI control registers, it effectively changes the pinMode of the NSS pin (PA4) so that its no longer an output pin (I’ve no idea what pin mode it becomes but its no longer a GPIO output)

We have found that a fix for this is to set pinMode(PA4,OUTPUT); after calling SPI.begin()

So, I think the solution is to modify SPI.begin() to do this as part of its operation.

However, my concern is that if someone has decided to use PA4 as an input and attached it to a switch etc, then if the code unilaterally sets it to OUTPUT in SPI.begin() it could damage the hardware.

So, I think the solution is to get the pinMode of PA4 at the start of SPI.begin() and then after the SPI reg’s have been setup, to re-instate the pinMode of PA4. i.e if it has been set to OUTPUT prior to SPI.begin() being called, it will be restored to that pinMode before begin() returns.

Note, for SPI2 and SPI3 there are different NSS pins,so the code (in begin() ) needs to check which SPI channel is being use and only restore the relevant NSS pin’s pinMode

If anyone can see a flaw in this, please let me know


RogerClark
Tue Jun 09, 2015 11:45 am
Guys,

I can’t finish this tonight, as its getting too late for me

But here is the work so far

The first function is the existing one that sets the GPIO mode (well its the code that changes the GPIO CR and ODR registers)

The function below it, is my attempt to do the opposite

void gpio_set_mode(gpio_dev *dev, uint8 pin, gpio_pin_mode mode) {
gpio_reg_map *regs = dev->regs;
__io uint32 *cr = &regs->CRL + (pin >> 3);
uint32 shift = (pin & 0x7) * 4;
uint32 tmp = *cr;

tmp &= ~(0xF << shift);
tmp |= (mode == GPIO_INPUT_PU ? GPIO_INPUT_PD : mode) << shift;
*cr = tmp;

if (mode == GPIO_INPUT_PD) {
regs->ODR &= ~(1U << pin);
} else if (mode == GPIO_INPUT_PU) {
regs->ODR |= (1U << pin);
}
}

gpio_pin_mode gpio_get_mode(gpio_dev *dev, uint8 pin) {
gpio_reg_map *regs = dev->regs;
__io uint32 *cr = &regs->CRL + (pin >> 3);
uint32 shift = (pin & 0x7) * 4;
uint32 tmp = *cr;

uint32 crMode = (*cr>>shift) & 0x0F;

// could be pull up or pull down. Nee to check the ODR
if (crMode==GPIO_INPUT_PD && ((regs->ODR >> pin) & 0x01) !=0 )
{
crMode = GPIO_INPUT_PU;
}

return(crMode);
}
}


victor_pv
Tue Jun 09, 2015 2:12 pm
Roger, I understand the function spi_config_gpios in STM32F1/cores/maple/libmaple/spi_f1.c sets the NSS pin as AF:
gpio_set_mode(nss_dev, nss_bit, GPIO_AF_OUTPUT_PP);

That function gets called in HardwareSPI by configure_gpios, which is turn is called by enable_device, which is called by void HardwareSPI::begin

So we can remap the file back to the normal GPIO function like you propose above, or we can just stop changin that that pin to the Alternate Function if setting it as AF doesn’t make it work as NSS anyway due to a hardware bug.

Also if we change the function spi_config_gpio to not do the gpio_set_mode in the NSS pin, it will work for all SPI ports.


pico
Tue Jun 09, 2015 2:21 pm
RogerClark wrote:
We have found that a fix for this is to set pinMode(PA4,OUTPUT); after calling SPI.begin()

ahull
Tue Jun 09, 2015 2:27 pm
If you could be sure of the timings, it might be possible to leverage the timers or even the PWM function to assert and de-assert the pin….. just mulling this over in my mind at the moment….

pico
Tue Jun 09, 2015 2:30 pm
ahull wrote:If you could be sure of the timings, it might be possible to leverage the timers or even the PWM function to assert and de-assert the pin….. just mulling this over in my mind at the moment….

Vassilis
Tue Jun 09, 2015 3:18 pm
I made some modifications to files
\Arduino_STM32\STM32F1\libraries\SPI\src\SPI.c and \Arduino_STM32\STM32F1\libraries\SPI\src\SPI.h

I added a new begin constructor.

SPI.h
/**
* EXPERIMENTAL CONSTRUCTOR (Vassilis Serasidis)
*/
void begin(int8_t pin, SPISettings settings);


RogerClark
Tue Jun 09, 2015 9:02 pm
Victor

I didn’t realise that the code was remapping the NSS pin.

I wonder why Leaflabs did this. I agree this seems pointless.

Its worth commenting out this line and seeing what the NSS line does on PA4, but it sounds like the NSS has issues in the hardware regardless of what pin its mapped to.

@andy and @pic

We could easily put code into SPI.transfer to control the SS line, but its very inefficient to do it that way.

AFIK, normally on the Arduino platform, the SS pin is controlled by code in the SD and other libs, or by the code in a sketch.

Even if we managed to get the hardware NSS working , I’d have concerns there would be a clash with the libs controlling the pin via digitalWrite etc

@pico

I intend to do what you describe, but just setting PA4 to output could be harmful if someone has connected this pin to GND via a normally closed switch.

Which is why I want to get the pin mode of PA4 at the start of begin() and reinstate that mode at the end of begin()


mrburnette
Wed Jun 10, 2015 12:19 am
I intend to do what you describe, but just setting PA4 to output could be harmful if someone has connected this pin to GND via a normally closed switch.

IMO, this is a real concern because some online references tell users to “ground/GND” unused pins.
https://www.google.com/#q=Ground+unused … oller+pins
Once one get into the specifics, the references are usually qualified with “resistor or capacitor.” I have also seen such advice in various Arduino forum responses.

Ray


RogerClark
Wed Jun 10, 2015 12:44 am
Guys,

I’ve pushed a possibly fix for the SPI NSS / CS issue

see

https://github.com/rogerclarkmelbourne/ … fix_spi_ss

The code changes are to add a new low level function called gpio_get_mode() and to use this new function in SPI::Begin()

Here is the revised SPI::begin

Firstly the code has to determine which SPI Device is being used.
As we don’t store the SPI channel number, I have to compare the SPI device (address) with the add of the 3 possible SPI devices.
(This is no big deal, as its all just 32 bit numbers to the processor)

Of course I need to have the #ifdef’s to cater for different processor models – I took the #ifdef’s from the SPIClass constructor, but looking at them, I don’t think there are actually any boards with less than 2 SPI’s, so really the first 2 ifdefs in the SPIClass constructor and here in begin are really redundant. But anyway…..

After that, the code simply gets the gpio mode of the nss pin, then after the SPI hardware has been configure, it sets the pinmode back to its stored state.

Note. I’ve not looked at the AF REMAP that Victor pointed out. But it doesnt look like a pin remap, it looks like its a remap to make the pin “OUTPUT Push Pull” which is what we want, i.e a normal OUTPUT pin.
But we can come back to this

I’ve done some basic testing on one of my test rigs and SD seems to work OK, but I need to test it on a larger board to confirm its OK

void SPIClass::begin(void) {
gpio_pin_mode nssPinMode;
int nssPin;
if (dataMode >= 4) {
ASSERT(0);
return;
}

#if BOARD_NR_SPI >= 1
if (this->spi_d == SPI1)
{
nssPin=BOARD_SPI1_NSS_PIN;
}
#endif
#if BOARD_NR_SPI >= 2
if (this->spi_d == SPI2)
{
nssPin=BOARD_SPI2_NSS_PIN;
}
#endif
#if BOARD_NR_SPI >= 3
if (this->spi_d == SPI3)
{
nssPin=BOARD_SPI3_NSS_PIN;
}
#endif

nssPinMode = gpio_get_mode(PIN_MAP[nssPin].gpio_device, PIN_MAP[nssPin].gpio_bit);// get and save NSS pin mode

uint32 flags = ((bitOrder == MSBFIRST ? SPI_FRAME_MSB : SPI_FRAME_LSB) | SPI_DFF_8_BIT | SPI_SW_SLAVE | SPI_SOFT_SS);
spi_init(spi_d);
configure_gpios(spi_d, 1);
#ifdef SPI_DEBUG
Serial.print("spi_master_enable("); Serial.print(clockDivider); Serial.print(","); Serial.print(dataMode); Serial.print(","); Serial.print(flags); Serial.println(")");
#endif
spi_master_enable(spi_d, (spi_baud_rate)clockDivider, (spi_mode)dataMode, flags);
gpio_set_mode(PIN_MAP[nssPin].gpio_device, PIN_MAP[nssPin].gpio_bit, nssPinMode);// restore pin mode of nss pin (work around for bug in the STM32)
}


RogerClark
Wed Jun 10, 2015 12:56 am
Update

I’ve tested this with my F103ZET (albeit on SPI 1 / PA4) and looked at PA4 with my scope, and its working fine.

If anyone else can test the branch https://github.com/rogerclarkmelbourne/ … fix_spi_ss

It would be handy. But if I don’t get any negative feedback in 24 hours, I’ll merge it into master copy of the repo

Cheers

Roger


pico
Wed Jun 10, 2015 5:03 am
RogerClark wrote:Whats worse is that when the code sets up the SPI control registers, it effectively changes the pinMode of the NSS pin (PA4) so that its no longer an output pin (I’ve no idea what pin mode it becomes but its no longer a GPIO output)

RogerClark
Wed Jun 10, 2015 5:49 am
@pico

My main consideration was compatibility with existing libs. e.g. SD, where they control the SS pin using GPIO

If we did manage to get the automatic SS working (I’m not sure its even possible based on the threads on the STM32 forums), I think the problem is that we end up toggling that pin regardless of whether the user wants to toggle the pin.

If we want to use the Hardware NSS function, I think for general compatibility, we need to modify either the SPIClass constructor or the begin() function, with an extra defaulted argument e.g. NSSMode. i.e default NSSMode to GPIO for maximum compatibility, but let users set NSSMode to HardwareNSS if they specifically want this functionality.

Note. At the moment I’ve not touched the AF REMAP OUTPUT PP line, I will need to test what happens if I remove it, i.e does the NSS pin still operate as GPIO.

BTW.
Did you manage to get NSS toggle automatically ? When I tried setting the MSS bit in the CR it didnt seem to make the NSS pin active. But I guess the AF REMAP stuff may be causing problems with this


pico
Wed Jun 10, 2015 6:36 am
RogerClark wrote:@pico

My main consideration was compatibility with existing libs. e.g. SD, where they control the SS pin using GPIO

If we did manage to get the automatic SS working (I’m not sure its even possible based on the threads on the STM32 forums), I think the problem is that we end up toggling that pin regardless of whether the user wants to toggle the pin.

If we want to use the Hardware NSS function, I think for general compatibility, we need to modify either the SPIClass constructor or the begin() function, with an extra defaulted argument e.g. NSSMode. i.e default NSSMode to GPIO for maximum compatibility, but let users set NSSMode to HardwareNSS if they specifically want this functionality.


RogerClark
Wed Jun 10, 2015 6:48 am
@pico,

When I re-read your posting, I realized we’d proposed the same default mode.

Re: Auto toggling

Do you mean change SPI.transfer to handle toggling the CS using GPIO ?

I think the only issue with this, is that this is time critical code. If we put if’s in there which toggle the CS then I’m sure people will complain to me that its been slowed down by adding a new feature – there is a huge amount of resistance to anything percieved to slowing things down.
( if statements seem to cause slowdown on the STM32, because of the instruction pipeline caching)

Which is understandable, because most people are using the STM32 because its 72Mhz and 32 bit, hence around 5 x faster than AVR.

I suspect we’d need to add new functions e.g. transferHWNSS()

Unfortunately Its not possible to do conditional compile to turn this on and off.

We already have a new function which we added SPIClass::dmaTransfer so this could easily be modified to handle its own CS if necessary, but currently its blocking so automatic CS isn’t such an issue.

I also vaguely some problem with adding a DMA callback function pointer to the SPIClass, but perhaps I’m confusing it with something else.
umm. Perhaps it was to do with which SPI channel was being used, and how the DMA callbacks need to be attached to different vectors… Unfortunately my memory is a bit hazy about this as it was a few months ago I looked at it :-(


pico
Wed Jun 10, 2015 6:58 am
RogerClark wrote:When I tried setting the MSS bit in the CR it didnt seem to make the NSS pin active. But I guess the AF REMAP stuff may be causing problems with this

RogerClark
Wed Jun 10, 2015 7:12 am
@pico

Ah.

I see what you mean.

I was using the define for the pin from the header, and I just did a binary or | onto the end of the existing pattern.

Strangely I thought when I looked at the patten as hex it looked OK, but I was probably deceiving myself

But like you say, it breaks SPI completely if you just do that ;-)


pico
Wed Jun 10, 2015 7:55 am
RogerClark wrote:@pico,

When I re-read your posting, I realized we’d proposed the same default mode.

Re: Auto toggling

Do you mean change SPI.transfer to handle toggling the CS using GPIO ?

I think the only issue with this, is that this is time critical code. If we put if’s in there which toggle the CS then I’m sure people will complain to me that its been slowed down by adding a new feature – there is a huge amount of resistance to anything percieved to slowing things down.
( if statements seem to cause slowdown on the STM32, because of the instruction pipeline caching)

Which is understandable, because most people are using the STM32 because its 72Mhz and 32 bit, hence around 5 x faster than AVR.


RogerClark
Wed Jun 10, 2015 8:02 am
@pico

No worries

If possible can you test the code in the fix_spi_ss branch and let me know if the GPIO is working OK for you on PA4 , ie check the save and restore pin mode stuff is working in SPIClass::begin

I was hoping for a few more people to test it, but I will need to wait until tomorrow to give the guys in Europe and the US time to read this thread and test things.

Thanks

Roger


pico
Wed Jun 10, 2015 9:51 am
RogerClark wrote:@pico
If possible can you test the code in the fix_spi_ss branch and let me know if the GPIO is working OK for you on PA4 , ie check the save and restore pin mode stuff is working in SPIClass::begin

RogerClark
Wed Jun 10, 2015 10:08 am
@pico

.

I will give that a try.

I will probably keep my new gpio_get_mode function as it may come in handy ;-), but I”ll not use it in SPIClass::Begin if simply commenting out the AF REMAP line works

Thanks


RogerClark
Wed Jun 10, 2015 10:26 am
@pico

Did you look at PA4 with a logic analyser ?

I made commented out that line, but what it appears to do is to force PA4 low all the time.

My scope is showing about 280mV and some noise on PA4 all the time.

My SD Card works fine, but thats because its the only device attached to SPI 1, as I’m running a VS1053 on SPI 2 so there is no clash.

Edit.

Hang on

I’m not sure my scope is working ;-)

Perhaps this fix is OK.

Edit 2

Apologies, it is working.

Not a scope problem, a sketch problem.

It does appear to work. i.e just commenting out that line. I can now see some nice square pulses on my scope !

Edit 3.

I just pushed the new version to that branch fix_spi_nss

I removed the pin mode saving and resetting code from SPI.c and just commented out the AF REMAP line in spi_f1.c


pico
Wed Jun 10, 2015 11:04 am
RogerClark wrote:I’m not sure my scope is working ;-)

ahull
Wed Jun 10, 2015 12:17 pm
pico wrote:RogerClark wrote:I’m not sure my scope is working ;-)

RogerClark
Wed Jun 10, 2015 9:52 pm
Hi andy

In this case, fortunately, it was a software error.

I had been running with CS on PA2 which on my board is next to PA4.

Then yesterday when I was trying to get NSS (PA4) to work, I moved CS and changed the sketch.

But I could not have saved the change.

So when I tried it again, the next day CS was set to PA2 and my scope was on PA4, hence nothing was seen on PA4.

Touch wood.. I have not blown anything up recently, but after you posted about your magic smoke incident, I’m now a bit more carefully, especially about metal from USB plugs, or other boards, shorting out pins on the board I have powered up.

I’ve also taken to disconnecting the USB (and hence power) when I connect the scope etc, as there is always a chance I can sort adjacent pins when connecting the scope.


Leave a Reply

Your email address will not be published. Required fields are marked *