Merge branch 'bugfix/spi_timing_issues' into 'master'

Fix timing adjustment needed for higher speeds of SPI master bus.

Ref https://github.com/espressif/esp-idf/issues/363 . It was found out the master SPI driver didn't exactly calculate the delay compensation needed, breaking 20 and 26MHz full-duplex mode. This fixes these use cases. We also found out 40MHz full-duplex routed over the GPIO matrix does not work because of a hardware quirk; this MR adds a check/error for that case until we find a workaround.

See merge request !547
This commit is contained in:
Jeroen Domburg 2017-03-31 10:44:37 +08:00
commit a4acf7b67b
3 changed files with 71 additions and 32 deletions

View File

@ -153,6 +153,9 @@ esp_err_t spi_bus_free(spi_host_device_t host);
* peripheral and routes it to the indicated GPIO. All SPI master devices have three CS pins and can thus control
* up to three devices.
*
* @note While in general, speeds up to 80MHz on the dedicated SPI pins and 40MHz on GPIO-matrix-routed pins are
* supported, full-duplex transfers routed over the GPIO matrix only support speeds up to 26MHz.
*
* @param host SPI peripheral to allocate device on
* @param dev_config SPI interface protocol config for the device
* @param handle Pointer to variable to hold the device handle

View File

@ -268,6 +268,8 @@ esp_err_t spi_bus_initialize(spi_host_device_t host, spi_bus_config_t *bus_confi
spihost[host]->hw->dma_out_link.start=0;
spihost[host]->hw->dma_in_link.start=0;
spihost[host]->hw->dma_conf.val&=~(SPI_OUT_RST|SPI_AHBM_RST|SPI_AHBM_FIFO_RST);
//Reset timing
spihost[host]->hw->ctrl2.val=0;
//Disable unneeded ints
spihost[host]->hw->slave.rd_buf_done=0;
@ -315,6 +317,7 @@ esp_err_t spi_bus_free(spi_host_device_t host)
esp_err_t spi_bus_add_device(spi_host_device_t host, spi_device_interface_config_t *dev_config, spi_device_handle_t *handle)
{
int freecs;
int apbclk=APB_CLK_FREQ;
SPI_CHECK(host>=SPI_HOST && host<=VSPI_HOST, "invalid host", ESP_ERR_INVALID_ARG);
SPI_CHECK(spihost[host]!=NULL, "host not initialized", ESP_ERR_INVALID_STATE);
SPI_CHECK(dev_config->spics_io_num < 0 || GPIO_IS_VALID_OUTPUT_GPIO(dev_config->spics_io_num), "spics pin invalid", ESP_ERR_INVALID_ARG);
@ -327,6 +330,9 @@ esp_err_t spi_bus_add_device(spi_host_device_t host, spi_device_interface_config
//The hardware looks like it would support this, but actually setting cs_ena_pretrans when transferring in full
//duplex mode does absolutely nothing on the ESP32.
SPI_CHECK(dev_config->cs_ena_pretrans==0 || (dev_config->flags & SPI_DEVICE_HALFDUPLEX), "cs pretrans delay incompatible with full-duplex", ESP_ERR_INVALID_ARG);
//Speeds >=40MHz over GPIO matrix needs a dummy cycle, but these don't work for full-duplex connections.
SPI_CHECK(!( ((dev_config->flags & SPI_DEVICE_HALFDUPLEX)==0) && (dev_config->clock_speed_hz > ((apbclk*2)/5)) && (!spihost[host]->no_gpio_matrix)),
"No speeds >26MHz supported for full-duplex, GPIO-matrix SPI transfers", ESP_ERR_INVALID_ARG);
//Allocate memory for device
spi_device_t *dev=malloc(sizeof(spi_device_t));
@ -394,8 +400,12 @@ static int spi_freq_for_pre_n(int fapb, int pre, int n) {
return (fapb / (pre * n));
}
static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) {
int pre, n, h, l;
/*
* Set the SPI clock to a certain frequency. Returns the effective frequency set, which may be slightly
* different from the requested frequency.
*/
static int spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) {
int pre, n, h, l, eff_clk;
//In hw, n, h and l are 1-64, pre is 1-8K. Value written to register is one lower than used value.
if (hz>((fapb/4)*3)) {
@ -405,6 +415,7 @@ static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) {
hw->clock.clkcnt_n=0;
hw->clock.clkdiv_pre=0;
hw->clock.clk_equ_sysclk=1;
eff_clk=fapb;
} else {
//For best duty cycle resolution, we want n to be as close to 32 as possible, but
//we also need a pre/n combo that gets us as close as possible to the intended freq.
@ -440,7 +451,9 @@ static void spi_set_clock(spi_dev_t *hw, int fapb, int hz, int duty_cycle) {
hw->clock.clkdiv_pre=pre-1;
hw->clock.clkcnt_h=h-1;
hw->clock.clkcnt_l=l-1;
eff_clk=spi_freq_for_pre_n(fapb, pre, n);
}
return eff_clk;
}
@ -516,14 +529,28 @@ static void IRAM_ATTR spi_intr(void *arg)
//Assumes a hardcoded 80MHz Fapb for now. ToDo: figure out something better once we have
//clock scaling working.
int apbclk=APB_CLK_FREQ;
spi_set_clock(host->hw, apbclk, dev->cfg.clock_speed_hz, dev->cfg.duty_cycle_pos);
int effclk=spi_set_clock(host->hw, apbclk, dev->cfg.clock_speed_hz, dev->cfg.duty_cycle_pos);
//Configure bit order
host->hw->ctrl.rd_bit_order=(dev->cfg.flags & SPI_DEVICE_RXBIT_LSBFIRST)?1:0;
host->hw->ctrl.wr_bit_order=(dev->cfg.flags & SPI_DEVICE_TXBIT_LSBFIRST)?1:0;
//Configure polarity
//SPI iface needs to be configured for a delay unless it is not routed through GPIO and clock is >=apb/2
int nodelay=(host->no_gpio_matrix && dev->cfg.clock_speed_hz >= (apbclk/2));
//SPI iface needs to be configured for a delay in some cases.
int nodelay=0;
int extra_dummy=0;
if (host->no_gpio_matrix) {
if (effclk >= apbclk/2) {
nodelay=1;
}
} else {
if (effclk >= apbclk/2) {
nodelay=1;
extra_dummy=1; //Note: This only works on half-duplex connections. spi_bus_add_device checks for this.
} else if (effclk >= apbclk/4) {
nodelay=1;
}
}
if (dev->cfg.mode==0) {
host->hw->pin.ck_idle_edge=0;
host->hw->user.ck_out_edge=0;
@ -543,11 +570,11 @@ static void IRAM_ATTR spi_intr(void *arg)
}
//Configure bit sizes, load addr and command
host->hw->user.usr_dummy=(dev->cfg.dummy_bits)?1:0;
host->hw->user.usr_dummy=(dev->cfg.dummy_bits+extra_dummy)?1:0;
host->hw->user.usr_addr=(dev->cfg.address_bits)?1:0;
host->hw->user.usr_command=(dev->cfg.command_bits)?1:0;
host->hw->user1.usr_addr_bitlen=dev->cfg.address_bits-1;
host->hw->user1.usr_dummy_cyclelen=dev->cfg.dummy_bits-1;
host->hw->user1.usr_dummy_cyclelen=dev->cfg.dummy_bits+extra_dummy-1;
host->hw->user2.usr_command_bitlen=dev->cfg.command_bits-1;
//Configure misc stuff
host->hw->user.doutdin=(dev->cfg.flags & SPI_DEVICE_HALFDUPLEX)?0:1;

View File

@ -80,41 +80,29 @@ TEST_CASE("SPI Master clockdiv calculation routines", "[spi]")
}
TEST_CASE("SPI Master test", "[spi][ignore]")
{
spi_bus_config_t buscfg={
.mosi_io_num=4,
.miso_io_num=16,
.sclk_io_num=25,
.quadwp_io_num=-1,
.quadhd_io_num=-1
};
static void test_spi_bus_speed(int hz) {
esp_err_t ret;
spi_device_handle_t handle;
spi_device_interface_config_t devcfg={
.command_bits=8,
.address_bits=64,
.dummy_bits=0,
.clock_speed_hz=8000,
.clock_speed_hz=hz,
.duty_cycle_pos=128,
.mode=0,
.spics_io_num=21,
.queue_size=3
.queue_size=3,
};
esp_err_t ret;
spi_device_handle_t handle;
printf("THIS TEST NEEDS A JUMPER BETWEEN IO4 AND IO16\n");
ret=spi_bus_initialize(HSPI_HOST, &buscfg, 1);
TEST_ASSERT(ret==ESP_OK);
ret=spi_bus_add_device(HSPI_HOST, &devcfg, &handle);
TEST_ASSERT(ret==ESP_OK);
printf("Bus/dev inited.\n");
spi_transaction_t t;
char sendbuf[16]="Hello World!";
char recvbuf[16]="UUUUUUUUUUUUUUU";
char sendbuf[64]="Hello World!";
char recvbuf[64]="UUUUUUUUUUUUUUU";
memset(&t, 0, sizeof(t));
t.length=16*8;
t.length=64*8;
t.tx_buffer=sendbuf;
t.rx_buffer=recvbuf;
t.address=0xA00000000000000FL;
@ -130,11 +118,32 @@ TEST_CASE("SPI Master test", "[spi][ignore]")
ret=spi_bus_remove_device(handle);
TEST_ASSERT(ret==ESP_OK);
ret=spi_bus_free(HSPI_HOST);
TEST_ASSERT(ret==ESP_OK);
TEST_ASSERT_EQUAL_INT8_ARRAY(sendbuf, recvbuf, 16);
TEST_ASSERT_EQUAL_INT8_ARRAY(sendbuf, recvbuf, 64);
}
TEST_CASE("SPI Master test", "[spi][ignore]")
{
spi_bus_config_t buscfg={
.mosi_io_num=4,
.miso_io_num=16,
.sclk_io_num=25,
.quadwp_io_num=-1,
.quadhd_io_num=-1
};
esp_err_t ret;
printf("THIS TEST NEEDS A JUMPER BETWEEN IO4 AND IO16\n");
ret=spi_bus_initialize(HSPI_HOST, &buscfg, 1);
TEST_ASSERT(ret==ESP_OK);
int freqs[]={8000, 1000000, 5000000, 10000000, 20000000, 26666666, 0};
for (int x=0; freqs[x]!=0; x++) {
printf("Testing clock speed of %dHz...\n", freqs[x]);
test_spi_bus_speed(freqs[x]);
}
ret=spi_bus_free(HSPI_HOST);
TEST_ASSERT(ret==ESP_OK);
}