Skip to content
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.

Sign up for GitHub

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

cpu/esp8266: allow arbitrary SPI clocks #20410

Merged
merged 1 commit into from
Mar 26, 2024
Merged

cpu/esp8266: allow arbitrary SPI clocks #20410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions cpu/esp8266/include/periph_cpu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "eagle_soc.h"
#include "cpu_conf.h"
#include "macros/units.h"

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -96,16 +97,16 @@
* @name Predefined GPIO names
* @{
*/
#define GPIO0 (GPIO_PIN(PORT_GPIO,0))

Check warning on line 100 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO1 (GPIO_PIN(PORT_GPIO,1))

Check warning on line 101 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO2 (GPIO_PIN(PORT_GPIO,2))

Check warning on line 102 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO3 (GPIO_PIN(PORT_GPIO,3))

Check warning on line 103 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO4 (GPIO_PIN(PORT_GPIO,4))

Check warning on line 104 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO5 (GPIO_PIN(PORT_GPIO,5))

Check warning on line 105 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO6 (GPIO_PIN(PORT_GPIO,6))

Check warning on line 106 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO7 (GPIO_PIN(PORT_GPIO,7))

Check warning on line 107 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO8 (GPIO_PIN(PORT_GPIO,8))

Check warning on line 108 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO9 (GPIO_PIN(PORT_GPIO,9))

Check warning on line 109 in cpu/esp8266/include/periph_cpu.h

View workflow job for this annotation

GitHub Actions / static-tests 

comma should be followed by whitespace
#define GPIO10 (GPIO_PIN(PORT_GPIO,10))
#define GPIO11 (GPIO_PIN(PORT_GPIO,11))
#define GPIO12 (GPIO_PIN(PORT_GPIO,12))
Expand Down Expand Up @@ -264,6 +265,19 @@
HSPI = 1, /**< HSPI interface controller */
} spi_ctrl_t;

/**
* @brief Override SPI clock speed values
* @{
*/
#define HAVE_SPI_CLK_T
typedef enum {
SPI_CLK_100KHZ = KHZ(100), /**< drive the SPI bus with 100KHz */
SPI_CLK_400KHZ = KHZ(400), /**< drive the SPI bus with 400KHz */
SPI_CLK_1MHZ = MHZ(1), /**< drive the SPI bus with 1MHz */
SPI_CLK_5MHZ = MHZ(5), /**< drive the SPI bus with 5MHz */
SPI_CLK_10MHZ = MHZ(10), /**< drive the SPI bus with 10MHz */
} spi_clk_t;

Comment on lines +268 to +280
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this change necessary if the enum values are (no longer?) used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enum values only go from 0 to 4, with this you can just write the desired SPI frequency to the clk parameter instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the original enum definition? I somehow doubt it is a good idea to override those only for a single platform?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous definition was here - you will find that overwriting this is done by most platforms already since the original interface is pretty bad/limiting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, maybe worth an issue to clean up the original interface then in a subsequent PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might introduce a new feature for platforms where you can select arbitrary SPI clocks, so drivers can rely on this.

/**
* @brief SPI configuration structure type
*/
Expand Down
27 changes: 5 additions & 22 deletions cpu/esp8266/periph/spi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,28 +262,11 @@ void IRAM_ATTR spi_acquire(spi_t bus, spi_cs_t cs, spi_mode_t mode, spi_clk_t cl
* https://www.espressif.com/sites/default/files/documentation/esp8266-technical_reference_en.pdf
*/

uint32_t spi_clkdiv_pre;
uint32_t spi_clkcnt_N;

switch (clk) {
case SPI_CLK_10MHZ: spi_clkdiv_pre = 2; /* predivides 80 MHz to 40 MHz */
spi_clkcnt_N = 4; /* 4 cycles results into 10 MHz */
break;
case SPI_CLK_5MHZ: spi_clkdiv_pre = 2; /* predivides 80 MHz to 40 MHz */
spi_clkcnt_N = 8; /* 8 cycles results into 5 MHz */
break;
case SPI_CLK_1MHZ: spi_clkdiv_pre = 2; /* predivides 80 MHz to 40 MHz */
spi_clkcnt_N = 40; /* 40 cycles results into 1 MHz */
break;
case SPI_CLK_400KHZ: spi_clkdiv_pre = 20; /* predivides 80 MHz to 4 MHz */
spi_clkcnt_N = 10; /* 10 cycles results into 400 kHz */
break;
case SPI_CLK_100KHZ: spi_clkdiv_pre = 20; /* predivides 80 MHz to 4 MHz */
spi_clkcnt_N = 40; /* 20 cycles results into 100 kHz */
break;
default: spi_clkdiv_pre = 20; /* predivides 80 MHz to 4 MHz */
spi_clkcnt_N = 40; /* 20 cycles results into 100 kHz */
}
uint32_t spi_clkdiv_pre; /* 13 bit */
uint32_t spi_clkcnt_N; /* 6 bit */

spi_clkcnt_N = 2;
spi_clkdiv_pre = (MHZ(80)/spi_clkcnt_N) / clk;

/* register values are set to deviders-1 */
spi_clkdiv_pre--;
Expand Down
Loading