Skip to content

atmega_common/periph/spi: bugfix for CLOCK_CORECLOCK != 16MHz#16811

Closed
hugueslarrive wants to merge 3 commits intoRIOT-OS:masterfrom
hugueslarrive:spi_atmega
Closed

atmega_common/periph/spi: bugfix for CLOCK_CORECLOCK != 16MHz#16811
hugueslarrive wants to merge 3 commits intoRIOT-OS:masterfrom
hugueslarrive:spi_atmega

Conversation

@hugueslarrive
Copy link
Copy Markdown
Contributor

@hugueslarrive hugueslarrive commented Sep 3, 2021

Contribution description

In atmega_common/include/periph_cpu_common.h sti_clk_t enum was overridden with a kind of encoded register bits assuming a master clock speed of 16MHz.

This leads into a (CLOCK_CORECLOCK / 16MHz) ratio for SPI_CLK_* for boards boards like atmega1284p where CLOCK_CORECLOCK differ from 16MHz.

In addition, the first two values were greater than the named values which does not comply with the usual rule.

Testing procedure

With ENABLE_DEBUG set to 1, build tests/periph_spi for atmega1284p:

CLOCK_CORECLOCK (8000000UL)

2021-09-03 22:28:45,903 # [spi] spi_clk[0]:   100000 ->    62500 (8000000 / 128)  SPI2X: 0  SPR1: 1  SPR0: 1
2021-09-03 22:28:45,989 # [spi] spi_clk[1]:   400000 ->   250000 (8000000 /  32)  SPI2X: 1  SPR1: 1  SPR0: 0
2021-09-03 22:28:46,075 # [spi] spi_clk[2]:  1000000 ->  1000000 (8000000 /   8)  SPI2X: 1  SPR1: 0  SPR0: 1
2021-09-03 22:28:46,162 # [spi] spi_clk[3]:  5000000 ->  4000000 (8000000 /   2)  SPI2X: 1  SPR1: 0  SPR0: 0
2021-09-03 22:28:46,249 # [spi] spi_clk[4]: 10000000 ->  4000000 (8000000 /   2)  SPI2X: 1  SPR1: 0  SPR0: 0
Measured period on SCK pin
  • spi_clk[0]: 16 µs
  • spi_clk[1]: 4 µs
  • spi_clk[2]: 1 µs
  • spi_clk[3]: 250 ns
  • spi_clk[4]: 250 ns

CLOCK_CORECLOCK (10000000UL)

 #error Unsupported hardware timer frequency (XTIMER_HZ), missing XTIMER_SHIFT in board.h? See xtimer.h documentation for more info

CLOCK_CORECLOCK (16000000UL)

I do not have a 16MHz crystal so the serial connection was made at 4800 bauds and the period measurements were divided by 2.

2021-09-03 22:54:09,349 # [spi] spi_clk[0]:   100000 ->   125000 (16000000 / 128)  SPI2X: 0  SPR1: 1  SPR0: 1
2021-09-03 22:54:09,524 # [spi] spi_clk[1]:   400000 ->   250000 (16000000 /  64)  SPI2X: 0  SPR1: 1  SPR0: 0
2021-09-03 22:54:09,699 # [spi] spi_clk[2]:  1000000 ->  1000000 (16000000 /  16)  SPI2X: 0  SPR1: 0  SPR0: 1
2021-09-03 22:54:09,873 # [spi] spi_clk[3]:  5000000 ->  4000000 (16000000 /   4)  SPI2X: 0  SPR1: 0  SPR0: 0
2021-09-03 22:54:10,048 # [spi] spi_clk[4]: 10000000 ->  8000000 (16000000 /   2)  SPI2X: 1  SPR1: 0  SPR0: 0
Measured period on SCK pin
  • spi_clk[0]: 8 µs
  • spi_clk[1]: 4 µs
  • spi_clk[2]: 1 µs
  • spi_clk[3]: 250 ns
  • spi_clk[4]: 125 ns

CLOCK_CORECLOCK (20000000UL)

 #error Unsupported hardware timer frequency (XTIMER_HZ), missing XTIMER_SHIFT in board.h? See xtimer.h documentation for more info

CLOCK_CORECLOCK (32000000UL)

I do not have a 16MHz crystal so the serial connection was made at 2400 bauds and the period measurements were divided by 4.
2021-09-03 23:04:52,390 # [spi] spi_clk[0]:   100000 ->   250000 (32000000 / 128)  SPI2X: 0  SPR1: 1  SPR0: 1
2021-09-03 23:04:52,740 # [spi] spi_clk[1]:   400000 ->   250000 (32000000 / 128)  SPI2X: 0  SPR1: 1  SPR0: 1
2021-09-03 23:04:53,089 # [spi] spi_clk[2]:  1000000 ->  1000000 (32000000 /  32)  SPI2X: 1  SPR1: 1  SPR0: 0
2021-09-03 23:04:53,439 # [spi] spi_clk[3]:  5000000 ->  4000000 (32000000 /   8)  SPI2X: 1  SPR1: 0  SPR0: 1
2021-09-03 23:04:53,788 # [spi] spi_clk[4]: 10000000 ->  8000000 (32000000 /   4)  SPI2X: 0  SPR1: 0  SPR0: 0
Measured period on SCK pin
  • spi_clk[0]: 4 µs
  • spi_clk[1]: 4 µs
  • spi_clk[2]: 1 µs
  • spi_clk[3]: 250 ns
  • spi_clk[4]: 125 ns

Issues/PRs references

#16727 (comment)

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: AVR Platform: This PR/issue effects AVR-based platforms labels Sep 3, 2021
hugueslarrive added a commit to hugueslarrive/RIOT that referenced this pull request Sep 4, 2021
@hugueslarrive
tests/periph_spi: printing and testing SPI clock rates
1db60d3 
drivers/include/periph/spi.h: add a function
which return the effective SPI clock rate
for a requested clock frequency

cpu/stm32/periph/spi: implementation
cpu/atmega_common/periph/spi: implementation + bugfix (RIOT-OS#16811)
cpu/atxmega/periph/spi: implementation
cpu/cc2538/periph/spi: implementation
cpu/efm32/periph/spi: implementation
cpu/esp_common/periph/spi: implementation
cpu/fe310/periph/spi: implementation
cpu/kinetis/periph/spi: implementation
cpu/lm4f120/periph/spi: implementation
cpu/lpc23xx/periph/spi: implementation
cpu/msp430fxyz/periph/spi: implementation
cpu/nrf51/periph/spi: implementation
cpu/nrf52/periph/spi: implementation
cpu/qn908x/periph/spi: implementation
cpu/sam0_common/periph/spi: implementation
cpu/sam3/periph/spi: implementation
@hugueslarrive hugueslarrive mentioned this pull request Sep 4, 2021
Open
38 tasks
hugueslarrive added a commit to hugueslarrive/RIOT that referenced this pull request Sep 8, 2021
@hugueslarrive
atmega_common/periph/spi:
daf11da 
- adapted to the new API (RIOT-OS#15904)
- arbitrary bus speed support (improves RIOT-OS#16811)
@hugueslarrive
Copy link
Copy Markdown
Contributor Author

hugueslarrive commented Sep 9, 2021

Improved with arbitrary speed support in #16727.

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

Reviewers

@kYc0o kYc0o Awaiting requested review from kYc0o kYc0o is a code owner

@maribu maribu Awaiting requested review from maribu maribu is a code owner

@roberthartung roberthartung Awaiting requested review from roberthartung

Assignees

No one assigned

Labels

Area: cpu Area: CPU/MCU ports Platform: AVR Platform: This PR/issue effects AVR-based platforms

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@hugueslarrive