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.

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

[TW#19301] SPI MISO on input only pins (GPIO34-35-36-39) not working #1736

Closed
gdefoest opened this issue Mar 17, 2018 · 5 comments
Closed

[TW#19301] SPI MISO on input only pins (GPIO34-35-36-39) not working #1736

gdefoest opened this issue Mar 17, 2018 · 5 comments

Comments

@gdefoest
Copy link

gdefoest commented Mar 17, 2018

Hi all,

I am working on a design that uses an external SPI device (SX1278) with the MISO pin assigned to GPIO35. This is an input only pin so it means the MISO (Master Input Slave Output) should work if the ESP32 is the SPI master but it is not the case.

I am getting the following error in the boot sequence:
E (449) gpio: io_num=35 can only be input.

This is my init sequence:

#define PIN_NUM_MISO 35
#define PIN_NUM_MOSI 32
#define PIN_NUM_CLK  33
#define PIN_NUM_CS   5
esp_err_t ret;
spi_device_handle_t spi;

spi_bus_config_t buscfg={
    .miso_io_num=PIN_NUM_MISO,
    .mosi_io_num=PIN_NUM_MOSI,
    .sclk_io_num=PIN_NUM_CLK,
    .quadwp_io_num=-1,
    .quadhd_io_num=-1
};

spi_device_interface_config_t devcfg={
    .clock_speed_hz=1*1000*1000,               //Clock out at 1 MHz
    .mode=0,                                //SPI mode 0
    .spics_io_num=PIN_NUM_CS,               //CS pin
    .queue_size=7,                          //We want to be able to queue 7 transactions at a time
};

I see this in the driver (spi_common.c, line 271)

if (bus_config->miso_io_num >= 0) {
    PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->miso_io_num], PIN_FUNC_GPIO);
    gpio_set_direction(bus_config->miso_io_num, GPIO_MODE_INPUT_OUTPUT);
    gpio_matrix_out(bus_config->miso_io_num, io_signal[host].spiq_out, false, false);
    gpio_matrix_in(bus_config->miso_io_num, io_signal[host].spiq_in, false);
}

The issue comes from the fact we assign an input only pin in an INPUT_OUTPUT.
So I think the piece of code above should be this instead:

if (bus_config->miso_io_num >= 0) {
   PIN_FUNC_SELECT(GPIO_PIN_MUX_REG[bus_config->miso_io_num], PIN_FUNC_GPIO);
   if ( GPIO_IS_VALID_OUTPUT_GPIO(bus_config->miso_io_num) ) {
      // All pins except the input only are handled here!
       gpio_set_direction(bus_config->miso_io_num, GPIO_MODE_INPUT_OUTPUT);
       gpio_matrix_out(bus_config->miso_io_num, io_signal[host].spiq_out, false, false);
       gpio_matrix_in(bus_config->miso_io_num, io_signal[host].spiq_in, false);
    } else {
       // The input only Pins are handled here!
       gpio_set_direction(bus_config->miso_io_num, GPIO_MODE_INPUT);
       gpio_matrix_in(bus_config->miso_io_num, io_signal[host].spiq_in, false);
    }
}

What are your thoughts?

@negativekelvin
Copy link
Contributor

Yes it looks correct, but miso needs to be output in dual/quad mode or slave mode so it should return an error in those cases.

@FayeY FayeY changed the title SPI MISO on input only pins (GPIO34-35-36-39) not working [TW#19301] SPI MISO on input only pins (GPIO34-35-36-39) not working Mar 19, 2018
@mreutman
Copy link
Contributor

mreutman commented Mar 31, 2018

It looks like in issue #598 that half duplex is not even functioning, and isn't likely to be fixed any time soon. Given that having half duplex supported is precondition to being able to do quad/dual IO on SPI, it seems rather unfortunate to further reduce the capabilities of the SPI interface. Would be preferable in my humble opinion to error out on spi_bus_add_device() where the spi_device_interface_config_t is specified (and the configuring for normal, dual, or quad IO is provided) rather than on spi_bus_initialize().

@igrr igrr closed this as completed in 45f8bcf Apr 17, 2018
@ginkgm
Copy link
Collaborator

ginkgm commented Apr 23, 2018

@mreutman this idea is good, we'll add this.

@igrr igrr reopened this Apr 23, 2018
@ginkgm
Copy link
Collaborator

ginkgm commented Apr 23, 2018

@gdefoest we fixed this, please check whether it works.

@gdefoest
Copy link
Author

gdefoest commented Aug 4, 2018

I tested it again on be81d2c.
No issue anymore.
Thanks a lot for the fix!

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

No branches or pull requests

5 participants