Skip to content

Fix calculation of SPI_MIN_CLOCK_DIVIDER in SPI.h #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 31, 2018
Merged

Fix calculation of SPI_MIN_CLOCK_DIVIDER in SPI.h #292

merged 1 commit into from
Jan 31, 2018

Conversation

joseangeljimenez
Copy link
Contributor

The previous version of SPI.h works only for the SAMD21G18A variant and only for a 48 MHz system clock. For the rest of variants, it sets SPI_MIN_CLOCK_DIVIDER to 2, which is incorrect for a 48 MHz system clock.

The SAMD21 datasheet specifies a typical SPI SCK period (tSCK) of 42 ns, see "Table 36-48. SPI Timing Characteristics and Requirements", which translates into a maximum SPI clock of 23.8 MHz.

The new code conservatively sets the divider for a 12 MHz maximum SPI clock, taking into account any value for the system clock (not only 48 MHz). It also executes for other variants of the SAMD, not only SAMD21G18A.

Can you, please, review this patch?

The previous version of SPI.h works only for the SAMD21G18A variant and only for a 48 MHz system clock. For the rest of variants, it sets SPI_MIN_CLOCK_DIVIDER to 2, which is incorrect for a 48 MHz system clock.

The SAMD21 datasheet specifies a typical SPI SCK period (tSCK) of 42 ns, see "Table 36-48. SPI Timing Characteristics and Requirements", which translates into a maximum SPI clock of 23.8 MHz.

The new code conservatively sets the divider for a 12 MHz maximum SPI clock, taking into account any value for the system clock (not only 48 MHz). It also executes for other variants of the SAMD, not only SAMD21G18A.

Can you, please, review this patch?
@joseangeljimenez
Copy link
Contributor Author

joseangeljimenez commented Jan 31, 2018

Additional clarification:

  • 24 MHz can work absolutely fine (at least inside the MCU) and should not be considered overclocking. The SAMD21 datasheet specifies a typical SPI SCK period (tSCK) of 42 ns, which translates into a typical SPI clock of 23.8 MHz. Hence, a 24 MHz clock is within a 1 % tolerance with respect to the typical value given in the datasheet.

  • Despite the previous, in my pull request Fix calculation of SPI_MIN_CLOCK_DIVIDER in SPI.h #292, I conservatively kept 12 MHz as the maximum clock for the SPI as this was the historical behavior of this code in the ArduinoCore repo.

  • I guess the historical 12 MHz top is due to the potential issue that enthusiasts prototyping with "air wires" and breadboards, without a proper PCB and layout, will get a malfunctioning SPI bus at higher frequencies. But that is due to quick & dirty prototyping techniques not due to an issue with the SAMD chips.

  • My main goal for the above pull request is that the SPI divider is calculated in the same way no matter which exact SAMD variant is used.

@cmaglie cmaglie merged commit 4294d36 into arduino:master Jan 31, 2018
@cmaglie cmaglie added this to the Release 1.6.18 milestone Jan 31, 2018
@cmaglie
Copy link
Member

cmaglie commented Jan 31, 2018

I guess the historical 12 MHz top is due to the potential issue that enthusiasts prototyping with "air wires" and breadboards, without a proper PCB and layout, will get a malfunctioning SPI bus at higher frequencies. But that is due to quick & dirty prototyping techniques not due to an issue with the SAMD chips.

I remember that we did some tests with 24Mhz SPI clock, but turned out that the CLK front was off of some nanoseconds making it incompatible with Mode 3 devices. Moreover, at the times, an Atmel' engineer confirmed that the maximum clock speed for the SAMD21 is 12MHz.

BTW if your experience is different, and 24MHz is possible, I'd like to hear more about that.

@joseangeljimenez
Copy link
Contributor Author

My experience is positive with SPI clocks between 12 and 24 MHz, with good PCB layout, short & narrow (5-6 mils) tracks. I have not tested, however, a SAMD21 device above 24 MHz.

For that, you have to take into account the I/O pins electrical specifications and the external loading of the SPI pins, that's where PCB layout vs. breadboarding makes a huge difference. In short, the maximum reliable SPI clock heavily depends on the external loading.

According to the datasheet (Table 36-13. Normal I/O Pins Characteristics), the rise/fall times of the output SPI pins can be up to 15 ns for an external pin load capacitance of 5/20 nF, depending on the value of PORT.PINCFG.DRVSTR (pin driver). For DRVSTR = 0, a maximum capacitance of 5 pF should be present at the output SPI pins. That explains the CLK front delay you observed.

In short, to attain a reliable 24 MHz SPI communication, you will need less than 5 pF external capacitance (DRVSTR = 0) or 20 pF (DRVSTR = 1), which may be not feasible with breadboarding and/or long wires. It also depends on the load capacitance of the slave SPI chip pins.

On the one hand, being conservative and sticking to a 12 MHz max clock will help enthusiasts to continue using breadboarding with Arduino Zero without too much trouble. On the other hand, it will limit other designers willing to attain higher clocks under a controlled PCB layout setup.

@joseangeljimenez
Copy link
Contributor Author

Thanks to @ladyada for pointing out that the latest datasheet (rev. A dated 01/2017) clearly indicates a typical tSCK = 84 ns, which roughly translates into a maximum SPI clock of 12 MHz. That pretty much settles the matter.

P.S.: Atmel definitely changed this specification at some point. My old locally stored datasheet rev 42181D (dated 09/2014), indicates tSCK (typ.) = 42 ns (roughly 24 MHz). Wow!

@cmaglie
Copy link
Member

cmaglie commented Feb 1, 2018

P.S.: Atmel definitely changed this specification at some point. My old locally stored datasheet rev 42181D (dated 09/2014), indicates tSCK (typ.) = 42 ns (roughly 24 MHz). Wow!

Oh good to know, better late than never :-)

Thanks for sharing your experience anyway!

@gioreva
Copy link

gioreva commented Sep 10, 2020

Changing the line 45 of the SPI Library
https://github.com/arduino/ArduinoCore-samd/blob/master/libraries/SPI/SPI.h
on
#define SPI_MIN_CLOCK_DIVIDER (uint8_t)(1 + ((F_CPU - 1) / 24000000))

SPI is 24Mhz.
Signal, with oscilloscope are good.

https://snipboard.io/7f5uxT.jpg

The board is this:
https://shop.gevaelettronica.it/en/arduino-compatible/63-gevino-tft-7.html

SAMD21 connected ar RAIO 8876M SPI TFT controller and SD reader.

Signal is good, but devices do not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants