What would the best method to make libraries compatible with both ports, and make it easy to select which one to use?
The obvious easy way is to replace SPI.transfer etc for something like SPI_display.transfer, pretty much what everyone does with Serial.print, debugSerial.print, etc, and have a #define on the top of the header file, but is definitely not the most elegant. something such as:
#define SPI_display SPI
or
#define SPI_display SPI2
What other easy, mostly compatible ways we have, to make libraries compatible with both SPI ports, while at the same time not modifying them so much that existing sketches or examples need to be changed much. Anyone?
That is a big question …
The other thread viewtopic.php?f=13&t=53&start=30, which discussed about global instantiation and #ifdef bring the same question.
In my case, on my Netduino2Plus, I have 3 SPIs, one for SDCard, on for ENC28J60 and the third one is free to use on header pins.
I suggested to Roger to start another thread about this potential “can of worms”.
In the mean time, I think we don’t much choice to tweak core files according to our needs.
I suggested to Roger to start another thread about this potential “can of worms”.
In the mean time, I think we don’t much choice to tweak core files according to our needs.
I totally agree, and it is not only about the core files, but libraries too.
For example, how can we get the SD lib and the Ethernet lib running on separate SPIs at the same time if both are using the same global instance SPI ?
The link you provide simply shows that we can change pins, but nothing is mentionned about multiples instances.
This is really a “can of worms” that even Arduino community is facing, but no real answers to the question. 
It sure that we can copy/paste libs to workaround that, but it is really ugly.
I wish to have all controls within the sketch itself, but I don’t know if we can achieve that.
I suggested to Roger to start another thread about this potential “can of worms”.
In the mean time, I think we don’t much choice to tweak core files according to our needs.
In Energia, they have added a SPI.setModule() method for the arm cortex-m4f boards. This method lets you switch between an array of SPI devices. They have a table that maps the board pin numbers to the different SPI pins. So SPI.setModule(0); picks SPI1, SPI.setModule(1); picks SPI2. I haven’t looked at the details or actually used it.
-rick
Do you think it can be doable in the context of having everything done in the sketch and avoiding touching libs at all, including existing ones from Arduino ?
But how in the context of global constructor like Sd2Card for example ?
(Although it can work if everything is done in the begin() call by setup(), if nothing is done in the constructor itself, right ? …)
But how in the context of global constructor like Sd2Card for example ?
(Although it can work if everything is done in the begin() call by setup(), if nothing is done in the constructor itself, right ? …)
But the existing libs are guilty
But the existing libs are guilty
0rick
I just noticed this thread. ( I find it hard to keep track of which threads to spend my time reading, so if there is an important thread you think I should read, please feel free to PM me)
Global instantiation has been discussed a few times before, and there is no good solution.
Adding the CS pin stuff (due extended functionality), is a double edged sword. I didn’t think that the existing SPI libs on AVR controlled the CS pin at all.
Can someone confirm this ?
We can’t use the hardware NSS pin, as there is a bug / feature in the STM32 hardware that means its generally not usable.
We could put in extra code in SPI::transfer to set and reset a designated pin, but as transfer is a byte at a time, this would considerably slow down transfers, and just having an if statement to check if we should control the pin would also slow things down noticeably .
re: SPI::setModule()
This would be kind of slow.
We’d need to re instantiate SPI each time and possibly run all the hardware setup.
Well, I guess we could keep separate the lower and upper level of the current SPI class, so that the SPI class stores the data for multiple hardware SPI devices, e.g in an array, and switches which device it uses.
This would probably still retain the speed in SPI::transfer, but I’d need to look at the code to make sure
Or we ditch the global instantiation. But I can hear the screams from the newbies already, just by mentioning this.
Re: Slowness
Actually, I think I should have gone back and edited my post. It was more a stream of consciousness
I agree.
if we implement
SPIClass::setModule(int spi_num)
{
switch (spi_num) {
#if BOARD_NR_SPI >= 1
case 1:
this->spi_d = SPI1;
break;
#endif
#if BOARD_NR_SPI >= 2
case 2:
this->spi_d = SPI2;
break;
#endif
#if BOARD_NR_SPI >= 3
case 3:
this->spi_d = SPI3;
break;
#endif
default:
ASSERT(0);
}
}
Here is what they do:
Re: Pins and SPISettings,
those are already members of the SPI class, so it is not an issue, although it also need to become an array (depending of the implementation, because the current SPI class can be become an SPIport class and the SPI class can become parent with an array of SPIport)
For RAM consumption, there is maybe ways to not allocate them until the actual SPI.begin() been called for specific port number, which also leave the opportunity for backward compatibilty if SPI.setModule() is never called, it will always use the SPIdev[0].
It is sure that a lot of thinking still need to be done before starting such refactoring, but this can be a big milestone.
I can’t understand such brainstorming never became fruitful for boards such Mega or Due, the issue is there since years. 
As Victor said in the first post, pretty much what everyone does with Serial.print, debugSerial.print, how many times we faced that.
Recently, personally, to do the debugging of SD lib under Netduino2Plus, I had to replace all Serial.printf to SerialUSB.printf (but not commit them) …
So, if our recipe work for SPI, we can even apply the same to Serial … but that’s another thougth … 
Rick Kimball wrote:I think you are over thinking it …
Here is what they do:
static const unsigned long g_ulSSIPort[] = {
#ifdef TARGET_IS_BLIZZARD_RB1
GPIO_PORTA_BASE, GPIO_PORTF_BASE, GPIO_PORTB_BASE, GPIO_PORTD_BASE
#else
#ifdef __TM4C129XNCZAD__
GPIO_PORTA_BASE, GPIO_PORTB_BASE, GPIO_PORTD_BASE, GPIO_PORTF_BASE, GPIO_PORTG_BASE, GPIO_PORTQ_BASE
#endif
#ifdef __TM4C1294NCPDT__
GPIO_PORTA_BASE, GPIO_PORTB_BASE, GPIO_PORTD_BASE, GPIO_PORTF_BASE, GPIO_PORTQ_BASE
#endif
#endif
};
What is good to know is that you seems to day that is easy to do. 
looks like multiple arrays
static const unsigned long g_ulSSIPins[] = {
#ifdef TARGET_IS_BLIZZARD_RB1
GPIO_PIN_2 | GPIO_PIN_3 | GPIO_PIN_4 | GPIO_PIN_5,
GPIO_PIN_0 | GPIO_PIN_1 | GPIO_PIN_2 | GPIO_PIN_3,
GPIO_PIN_0 | GPIO_PIN_1 | GPIO_PIN_2 | GPIO_PIN_3
#else
#ifdef __TM4C129XNCZAD__
GPIO_PIN_2 | GPIO_PIN_3 | GPIO_PIN_4 | GPIO_PIN_5,
GPIO_PIN_5 | GPIO_PIN_4 | GPIO_PIN_4 | GPIO_PIN_5,
GPIO_PIN_3 | GPIO_PIN_2 | GPIO_PIN_1 | GPIO_PIN_0,
GPIO_PIN_3 | GPIO_PIN_2 | GPIO_PIN_1 | GPIO_PIN_0,
GPIO_PIN_7 | GPIO_PIN_6 | GPIO_PIN_5 | GPIO_PIN_4,
GPIO_PIN_0 | GPIO_PIN_1 | GPIO_PIN_2 | GPIO_PIN_3
#endif
#ifdef __TM4C1294NCPDT__
GPIO_PIN_2 | GPIO_PIN_3 | GPIO_PIN_4 | GPIO_PIN_5,
GPIO_PIN_5 | GPIO_PIN_4 | GPIO_PIN_4 | GPIO_PIN_5,
GPIO_PIN_3 | GPIO_PIN_2 | GPIO_PIN_1 | GPIO_PIN_0,
GPIO_PIN_3 | GPIO_PIN_2 | GPIO_PIN_1 | GPIO_PIN_0,
GPIO_PIN_0 | GPIO_PIN_1 | GPIO_PIN_2 | GPIO_PIN_3
#endif
#endif
};
I think the conclusion until now (but it is not concluded) is that we need SPI class becomes an array of SPIdev, right ?
It it uses this for all the low level access to the hardware
If that pointer was to a different hardware device, then the SPI class would interact with that SPI channel.
But the hardware device doesn’t store the bitOrder or the SPI mode or the clock divider, they are separate vars (properties) of the SPI class.
When you call begin, it has to set the bit Order and divider etc as they are part of the control register for the hardware.
It may be possible to read back the bit order and clock divider, so that when we set these values we only update the values we actually want to change, but I’d need to look at the code in detail.
because basically SPIClass:begin() is what does the initialization on the hardware.
I think at least to start with, its easier just to have use the SPISettings class / struct to store the data (or a struct that contains a pointer to the spi device struct and ints for bitOrder, clock divider and spi mode)
We could initially duplicate SPI and call it SPI_STM.h and then we can do independent testing.
(Note I could put this in a branch of the repo, but I find most people don’t use git on their machines, and just download the zip, so adding a tempoarary new SPI class folder is easier for people to test with)
But the hardware device doesn’t store the bitOrder or the SPI mode or the clock divider, they are separate vars (properties) of the SPI class.
When you call begin, it has to set the bit Order and divider etc as they are part of the control register for the hardware.
However, I think we could implement something like it without doing the begin() each time. Just as @victor_pc explained.The SPI class just needs to have a concept of current SPI peripheral in use. The setModule(x) would just change the currently active SPI device. … I’ll have to play with this some to see how well this might work. Heh .. and yeah we need to tie in transactions support too, for those people using mulitple SPI devices on one SPI peripheral.
-rick
Sorry. I think I didn’t explain my concerns very well.
Yes. The bit order etc are stored in the SPI control register.
Normally what happens is that there is one write to the control register to set up a whole bunch of things rather than doing multiple Read – Write cycles with appropriate masking to set individual bits of the register that set the various parameters e.g. bitOrder
I think the thing I was trying to work out the best way to do, was the initial setup.
Looking at the description of what SPI::Begin() does (according to Arduino.cc’s page)
Initializes the SPI bus by setting SCK, MOSI, and SS to outputs, pulling SCK and MOSI low, and SS high
What this omits to say is that it will also set up the default clock divider, and default mode and default bit order.
So we’d need to have those values stored in the SPI class so that they can be used in begin()
Well, that is, unless we can set those values in the SPI constructor, without turning the SPI on . (which we probably can)
But at the moment we can’t do stuff in the constructor because of the PIN MAP in RAM issue (which I still have not had time to get working)
If we fix PIN_MAP into Flash, we can setup defaults for divider, bit order and mode in the constructor, without actually enabling the SPI device (or activating the pins as SPI rather then GPIO)
Then we don’t need to store those vars, as we can modify the setBitOrder() etc func’s to just update the bits of the CR that need to be updated, rather than updating the whole CR in begin()
It will mean that what we do in begin() will need to change, so that it doesn’t change the bit order etc .
In regards to the use of setModule(). I think we are saying that we’d need to call setModule if you were using separate libraries that each used SPI and were hard coded to use SPI with no way to set another device.
I think this could still be relatively problematic, if any of these libs calls setBitOrder etc in their constructor. But at the moment libs that do that won’t work anyway. And no matter which way we do things, this would be an issue, because the order in which the constructors are called is not defined, and we’d not know the state of bitOrder that SPI1 has been left with.
Anyway, that aside.
I think that using setModule() is worth a go.
So we’d need to have those values stored in the SPI class so that they can be used in begin()
I think it is already done using SPISettings member. Althougth I’m not sure it is done early, and it should have “defaults” values in case EndUsers never set them.
I think that using setModule() is worth a go.
Yes !
We could initially duplicate SPI and call it SPI_STM.h and then we can do independent testing.
I agree on that too ! … and as soon we discover that new lib is backward compatible, we can simply do the switch over.
But the basic principal seems to be have an array of SPISettings and also a pointer to the current setting. add spi_d to SPISettings, and change the code that uses spi_d etc to use
_currentSetting->spi_d
etc
then add a few method of setModule(int num)
which does
_currentSetting = &_settings[num];
i may have to play around with the dereferencing e.g pointers stuff, but i think the basic principal should be ok.
i will keep you posted.
That way the dmaSend functions can get the TX and RX channel from there and be able to do DMA for both ports, rather than having SPI1 channels hardcoded as it is right now.
i was going to use the SPISettings class, which is already defined in SPI.h,
And… Yes, i can add vars for these. are they board ints ?
I’ve started the work to add setModule() to SPI, and it kind of works.
Well. I changed all the code over to use the settings in the SPISettings class (i.e struct), and the SD card is working fine.
However… strangely, on my test rig with had a VS1053 MP3 (SPI) board attached to SPI channel 2 (where I’m passing the pointer to SPI 2 it already).
It crashes completely as soon as I call begin on the VS1053 class.
I tried changing moving the VS1053 to use SPI1 (albeit the SD card is also on that channel) and it still crashes (well perhaps it gets stuck in a loop in the begin() in the VS1003 class
But, I’ll need to leave it until this evening, as I need to get on with the day job
i.e it does work with the SD lib, but I think we also are getting more issues due to PIN MAP not being initialised early enough, so this needs to be the next thing I need to get into the repo
i.e it does work with the SD lib, but I think we also are getting more issues due to PIN MAP not being initialised early enough, so this needs to be the next thing I need to get into the repo
