Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

spi: fix the bug of connecting SPI peripheral to read-only pins

Browse source 
The requirements of pin capabilites is different for spi master and
slave.  The master needs CS, SCLK, MOSI to be output-able, while slave
needs MISO to be output-able.

Previous code is for master only.

This commit allows to place other 3 pins than MISO on input-only pins
for slaves. Refactoring for spi_common is also included.

Resolves https://github.com/espressif/esp-idf/issues/2455
This commit is contained in:
Michael (XIAO Xufeng) 2019-02-11 14:17:31 +08:00
parent c278dcb707
commit f6015c29d5
2 changed files with 158 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

132
components/driver/spi_common.c
View file

@ -34,11 +34,18 @@
static const char *SPI_TAG = "spi";
#define SPI_CHECK(a, str, ret_val) \
#define SPI_CHECK(a, str, ret_val) do { \
if (!(a)) { \
ESP_LOGE(SPI_TAG,"%s(%d): %s", __FUNCTION__, __LINE__, str); \
return (ret_val); \
}
} \
} while(0)
#define SPI_CHECK_PIN(pin_num, pin_name, check_output) if (check_output) { \
SPI_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(pin_num), pin_name" not valid", ESP_ERR_INVALID_ARG); \
} else { \
SPI_CHECK(GPIO_IS_VALID_GPIO(pin_num), pin_name" not valid", ESP_ERR_INVALID_ARG); \
}
typedef struct spi_device_t spi_device_t;
@ -115,6 +122,22 @@ bool spicommon_dma_chan_free(int dma_chan)
return true;
}
static bool bus_uses_iomux_pins(spi_host_device_t host, const spi_bus_config_t* bus_config)
{
if (bus_config->sclk_io_num>=0 &&
bus_config->sclk_io_num != spi_periph_signal[host].spiclk_iomux_pin) return false;
if (bus_config->quadwp_io_num>=0 &&
bus_config->quadwp_io_num != spi_periph_signal[host].spiwp_iomux_pin) return false;
if (bus_config->quadhd_io_num>=0 &&
bus_config->quadhd_io_num != spi_periph_signal[host].spihd_iomux_pin) return false;
if (bus_config->mosi_io_num >= 0 &&
bus_config->mosi_io_num != spi_periph_signal[host].spid_iomux_pin) return false;
if (bus_config->miso_io_num>=0 &&
bus_config->miso_io_num != spi_periph_signal[host].spiq_iomux_pin) return false;
return true;
}
/*
Do the common stuff to hook up a SPI host to a bus defined by a bunch of GPIO pins. Feed it a host number and a
bus config struct and it'll set up the GPIO matrix and enable the device. If a pin is set to non-negative value,
@ -122,67 +145,70 @@ it should be able to be initialized.
*/
esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, const spi_bus_config_t *bus_config, int dma_chan, uint32_t flags, uint32_t* flags_o)
{
bool use_iomux = true;
uint32_t temp_flag=0;
bool quad_pins_exist = true;
//the MISO should be output capable in slave mode, or in DIO/QIO mode.
bool miso_output = !(flags&SPICOMMON_BUSFLAG_MASTER) || flags&SPICOMMON_BUSFLAG_DUAL;
//the MOSI should be output capble in master mode, or in DIO/QIO mode.
bool mosi_output = (flags&SPICOMMON_BUSFLAG_MASTER)!=0 || flags&SPICOMMON_BUSFLAG_DUAL;
//check pins existence and if the selected pins correspond to the iomux pins of the peripheral
bool miso_need_output;
bool mosi_need_output;
bool sclk_need_output;
if ((flags&SPICOMMON_BUSFLAG_MASTER) != 0) {
//initial for master
miso_need_output = ((flags&SPICOMMON_BUSFLAG_DUAL) != 0) ? true : false;
mosi_need_output = true;
sclk_need_output = true;
} else {
//initial for slave
miso_need_output = true;
mosi_need_output = ((flags&SPICOMMON_BUSFLAG_DUAL) != 0) ? true : false;
sclk_need_output = false;
}
const bool wp_need_output = true;
const bool hd_need_output = true;
//check pin capabilities
if (bus_config->sclk_io_num>=0) {
temp_flag |= SPICOMMON_BUSFLAG_SCLK;
SPI_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(bus_config->sclk_io_num), "sclk not valid", ESP_ERR_INVALID_ARG);
if (bus_config->sclk_io_num != spi_periph_signal[host].spiclk_iomux_pin) use_iomux = false;
} else {
SPI_CHECK((flags&SPICOMMON_BUSFLAG_SCLK)==0, "sclk pin required.", ESP_ERR_INVALID_ARG);
SPI_CHECK_PIN(bus_config->sclk_io_num, "sclk", sclk_need_output);
}
if (bus_config->quadwp_io_num>=0) {
SPI_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(bus_config->quadwp_io_num), "spiwp not valid", ESP_ERR_INVALID_ARG);
if (bus_config->quadwp_io_num != spi_periph_signal[host].spiwp_iomux_pin) use_iomux = false;
} else {
quad_pins_exist = false;
SPI_CHECK((flags&SPICOMMON_BUSFLAG_WPHD)==0, "spiwp pin required.", ESP_ERR_INVALID_ARG);
SPI_CHECK_PIN(bus_config->quadwp_io_num, "wp", wp_need_output);
}
if (bus_config->quadhd_io_num>=0) {
SPI_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(bus_config->quadhd_io_num), "spihd not valid", ESP_ERR_INVALID_ARG);
if (bus_config->quadhd_io_num != spi_periph_signal[host].spihd_iomux_pin) use_iomux = false;
} else {
quad_pins_exist = false;
SPI_CHECK((flags&SPICOMMON_BUSFLAG_WPHD)==0, "spihd pin required.", ESP_ERR_INVALID_ARG);
SPI_CHECK_PIN(bus_config->quadhd_io_num, "hd", hd_need_output);
}
//set flags for QUAD mode according to the existence of wp and hd
if (bus_config->quadhd_io_num >= 0 && bus_config->quadwp_io_num >= 0) temp_flag |= SPICOMMON_BUSFLAG_WPHD;
if (bus_config->mosi_io_num >= 0) {
temp_flag |= SPICOMMON_BUSFLAG_MOSI;
if (mosi_output) {
SPI_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(bus_config->mosi_io_num), "mosi not valid", ESP_ERR_INVALID_ARG);
} else {
SPI_CHECK(GPIO_IS_VALID_GPIO(bus_config->mosi_io_num), "mosi not valid", ESP_ERR_INVALID_ARG);
}
if (bus_config->mosi_io_num != spi_periph_signal[host].spid_iomux_pin) use_iomux = false;
} else {
SPI_CHECK((flags&SPICOMMON_BUSFLAG_MOSI)==0, "mosi pin required.", ESP_ERR_INVALID_ARG);
SPI_CHECK_PIN(bus_config->mosi_io_num, "mosi", mosi_need_output);
}
if (bus_config->miso_io_num>=0) {
temp_flag |= SPICOMMON_BUSFLAG_MISO;
if (miso_output) {
SPI_CHECK(GPIO_IS_VALID_OUTPUT_GPIO(bus_config->miso_io_num), "miso not valid", ESP_ERR_INVALID_ARG);
} else {
SPI_CHECK(GPIO_IS_VALID_GPIO(bus_config->miso_io_num), "miso not valid", ESP_ERR_INVALID_ARG);
}
if (bus_config->miso_io_num != spi_periph_signal[host].spiq_iomux_pin) use_iomux = false;
} else {
SPI_CHECK((flags&SPICOMMON_BUSFLAG_MISO)==0, "miso pin required.", ESP_ERR_INVALID_ARG);
SPI_CHECK_PIN(bus_config->miso_io_num, "miso", miso_need_output);
}
//set flags for DUAL mode according to output-capability of MOSI and MISO pins.
if ( (bus_config->mosi_io_num < 0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->mosi_io_num)) &&
(bus_config->miso_io_num < 0 || GPIO_IS_VALID_OUTPUT_GPIO(bus_config->miso_io_num)) ) {
temp_flag |= SPICOMMON_BUSFLAG_DUAL;
}
//set flags for QUAD mode according to the existence of wp and hd
if (quad_pins_exist) temp_flag |= SPICOMMON_BUSFLAG_WPHD;
//check iomux pins if required.
SPI_CHECK((flags&SPICOMMON_BUSFLAG_NATIVE_PINS)==0 || use_iomux, "not using iomux pins", ESP_ERR_INVALID_ARG);
//check if the selected pins correspond to the iomux pins of the peripheral
bool use_iomux = bus_uses_iomux_pins(host, bus_config);
if (use_iomux) temp_flag |= SPICOMMON_BUSFLAG_NATIVE_PINS;
uint32_t missing_flag = flags & ~temp_flag;
missing_flag &= ~SPICOMMON_BUSFLAG_MASTER;//don't check this flag
if (missing_flag != 0) {
//check pins existence
if (missing_flag & SPICOMMON_BUSFLAG_SCLK) ESP_LOGE(SPI_TAG, "sclk pin required.");
if (missing_flag & SPICOMMON_BUSFLAG_MOSI) ESP_LOGE(SPI_TAG, "mosi pin required.");
if (missing_flag & SPICOMMON_BUSFLAG_MISO) ESP_LOGE(SPI_TAG, "miso pin required.");
if (missing_flag & SPICOMMON_BUSFLAG_DUAL) ESP_LOGE(SPI_TAG, "not both mosi and miso output capable");
if (missing_flag & SPICOMMON_BUSFLAG_WPHD) ESP_LOGE(SPI_TAG, "both wp and hd required.");
if (missing_flag & SPICOMMON_BUSFLAG_NATIVE_PINS) ESP_LOGE(SPI_TAG, "not using iomux pins");
SPI_CHECK(missing_flag == 0, "not all required capabilities satisfied.", ESP_ERR_INVALID_ARG);
}
if (use_iomux) {
//All SPI iomux pin selections resolve to 1, so we put that here instead of trying to figure
@ -213,7 +239,7 @@ esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, const spi_bus_conf
//Use GPIO matrix
ESP_LOGD(SPI_TAG, "SPI%d use gpio matrix.", host );
if (bus_config->mosi_io_num >= 0) {
if (mosi_output || (temp_flag&SPICOMMON_BUSFLAG_DUAL)) {
if (mosi_need_output || (temp_flag&SPICOMMON_BUSFLAG_DUAL)) {
gpio_set_direction(bus_config->mosi_io_num, GPIO_MODE_INPUT_OUTPUT);
gpio_matrix_out(bus_config->mosi_io_num, spi_periph_signal[host].spid_out, false, false);
} else {
@ -223,7 +249,7 @@ esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, const spi_bus_conf
PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->mosi_io_num], FUNC_GPIO);
}
if (bus_config->miso_io_num >= 0) {
if (miso_output || (temp_flag&SPICOMMON_BUSFLAG_DUAL)) {
if (miso_need_output || (temp_flag&SPICOMMON_BUSFLAG_DUAL)) {
gpio_set_direction(bus_config->miso_io_num, GPIO_MODE_INPUT_OUTPUT);
gpio_matrix_out(bus_config->miso_io_num, spi_periph_signal[host].spiq_out, false, false);
} else {
@ -245,8 +271,12 @@ esp_err_t spicommon_bus_initialize_io(spi_host_device_t host, const spi_bus_conf
PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->quadhd_io_num], FUNC_GPIO);
}
if (bus_config->sclk_io_num >= 0) {
gpio_set_direction(bus_config->sclk_io_num, GPIO_MODE_INPUT_OUTPUT);
gpio_matrix_out(bus_config->sclk_io_num, spi_periph_signal[host].spiclk_out, false, false);
if (sclk_need_output) {
gpio_set_direction(bus_config->sclk_io_num, GPIO_MODE_INPUT_OUTPUT);
gpio_matrix_out(bus_config->sclk_io_num, spi_periph_signal[host].spiclk_out, false, false);
} else {
gpio_set_direction(bus_config->sclk_io_num, GPIO_MODE_INPUT);
}
gpio_matrix_in(bus_config->sclk_io_num, spi_periph_signal[host].spiclk_in, false);
PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->sclk_io_num], FUNC_GPIO);
}
@ -309,8 +339,12 @@ void spicommon_cs_initialize(spi_host_device_t host, int cs_io_num, int cs_num,
gpio_iomux_out(cs_io_num, FUNC_SPI, false);
} else {
//Use GPIO matrix
gpio_set_direction(cs_io_num, GPIO_MODE_INPUT_OUTPUT);
gpio_matrix_out(cs_io_num, spi_periph_signal[host].spics_out[cs_num], false, false);
if (GPIO_IS_VALID_OUTPUT_GPIO(cs_io_num)) {
gpio_set_direction(cs_io_num, GPIO_MODE_INPUT_OUTPUT);
gpio_matrix_out(cs_io_num, spi_periph_signal[host].spics_out[cs_num], false, false);
} else {
gpio_set_direction(cs_io_num, GPIO_MODE_INPUT);
}
if (cs_num == 0) gpio_matrix_in(cs_io_num, spi_periph_signal[host].spics_in, false);
PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[cs_io_num], FUNC_GPIO);
}

82
components/driver/test/test_spi_master.c
View file

@ -25,6 +25,14 @@
const static char TAG[] = "test_spi";
#define PIN_NUM_MISO HSPI_IOMUX_PIN_NUM_MISO
#define PIN_NUM_MOSI HSPI_IOMUX_PIN_NUM_MOSI
#define PIN_NUM_CLK HSPI_IOMUX_PIN_NUM_CLK
#define PIN_NUM_CS HSPI_IOMUX_PIN_NUM_CS
#define TEST_SPI_HOST HSPI_HOST
#define TEST_SLAVE_HOST VSPI_HOST
#define SPI_BUS_TEST_DEFAULT_CONFIG() {\
.miso_io_num=PIN_NUM_MISO, \
.mosi_io_num=PIN_NUM_MOSI,\
@ -44,6 +52,14 @@ const static char TAG[] = "test_spi";
.input_delay_ns = 62.5,\
}
//default device config for slave devices
#define SPI_SLAVE_TEST_DEFAULT_CONFIG() {\
.mode=0,\
.spics_io_num=PIN_NUM_CS,\
.queue_size=3,\
.flags=0,\
}
#define FUNC_SPI 1
#define FUNC_GPIO 2
@ -291,6 +307,65 @@ TEST_CASE("SPI Master test, interaction of multiple devs", "[spi][ignore]") {
destroy_spi_bus(handle1);
}
static esp_err_t test_master_pins(int mosi, int miso, int sclk, int cs)
{
esp_err_t ret;
spi_bus_config_t cfg = SPI_BUS_TEST_DEFAULT_CONFIG();
cfg.mosi_io_num = mosi;
cfg.miso_io_num = miso;
cfg.sclk_io_num = sclk;
spi_device_interface_config_t master_cfg = SPI_DEVICE_TEST_DEFAULT_CONFIG();
master_cfg.spics_io_num = cs;
ret = spi_bus_initialize(TEST_SPI_HOST, &cfg, 1);
if (ret != ESP_OK) return ret;
spi_device_handle_t spi;
ret = spi_bus_add_device(TEST_SPI_HOST, &master_cfg, &spi);
if (ret != ESP_OK) {
spi_bus_free(TEST_SPI_HOST);
return ret;
}
TEST_ASSERT(spi_bus_remove_device(spi) == ESP_OK);
TEST_ASSERT(spi_bus_free(TEST_SPI_HOST) == ESP_OK);
return ESP_OK;
}
static esp_err_t test_slave_pins(int mosi, int miso, int sclk, int cs)
{
esp_err_t ret;
spi_bus_config_t cfg = SPI_BUS_TEST_DEFAULT_CONFIG();
cfg.mosi_io_num = mosi;
cfg.miso_io_num = miso;
cfg.sclk_io_num = sclk;
spi_slave_interface_config_t slave_cfg = SPI_SLAVE_TEST_DEFAULT_CONFIG();
slave_cfg.spics_io_num = cs;
ret = spi_slave_initialize(TEST_SLAVE_HOST, &cfg, &slave_cfg, 1);
if (ret != ESP_OK) return ret;
spi_slave_free(TEST_SLAVE_HOST);
return ESP_OK;
}
TEST_CASE("spi placed on input-only pins", "[spi]")
{
TEST_ESP_OK(test_master_pins(PIN_NUM_MOSI, PIN_NUM_MISO, PIN_NUM_CLK, PIN_NUM_CS));
TEST_ASSERT(test_master_pins(34, PIN_NUM_MISO, PIN_NUM_CLK, PIN_NUM_CS)!=ESP_OK);
TEST_ESP_OK(test_master_pins(PIN_NUM_MOSI, 34, PIN_NUM_CLK, PIN_NUM_CS));
TEST_ASSERT(test_master_pins(PIN_NUM_MOSI, PIN_NUM_MISO, 34, PIN_NUM_CS)!=ESP_OK);
TEST_ASSERT(test_master_pins(PIN_NUM_MOSI, PIN_NUM_MISO, PIN_NUM_CLK, 34)!=ESP_OK);
TEST_ESP_OK(test_slave_pins(PIN_NUM_MOSI, PIN_NUM_MISO, PIN_NUM_CLK, PIN_NUM_CS));
TEST_ESP_OK(test_slave_pins(34, PIN_NUM_MISO, PIN_NUM_CLK, PIN_NUM_CS));
TEST_ASSERT(test_slave_pins(PIN_NUM_MOSI, 34, PIN_NUM_CLK, PIN_NUM_CS)!=ESP_OK);
TEST_ESP_OK(test_slave_pins(PIN_NUM_MOSI, PIN_NUM_MISO, 34, PIN_NUM_CS));
TEST_ESP_OK(test_slave_pins(PIN_NUM_MOSI, PIN_NUM_MISO, PIN_NUM_CLK, 34));
}
TEST_CASE("spi bus setting with different pin configs", "[spi]")
{
spi_bus_config_t cfg;
@ -678,13 +753,6 @@ static void master_deinit(spi_device_handle_t spi)
TEST_ESP_OK( spi_bus_free(HSPI_HOST) );
}
#define SPI_SLAVE_TEST_DEFAULT_CONFIG() {\
.mode=0,\
.spics_io_num=PIN_NUM_CS,\
.queue_size=3,\
.flags=0,\
}
static void slave_pull_up(const spi_bus_config_t* cfg, int spics_io_num)
{
gpio_set_pull_mode(cfg->mosi_io_num, GPIO_PULLUP_ENABLE);