gh-raspberrypi/pico-sdk

Watch

Fork 0

spi_set_format does not update the clock polarity until after a read or write #868

New issue

Open

opened 2022-06-15 16:47:53 -05:00 by UKTailwind · 10 comments

UKTailwind commented

2022-06-15 16:47:53 -05:00

(Migrated from github.com)

Corrected code in spi.h

static inline void spi_set_format(spi_inst_t *spi, uint data_bits, spi_cpol_t cpol, spi_cpha_t cpha, __unused spi_order_t order) {
    invalid_params_if(SPI, data_bits < 4 || data_bits > 16);
    // LSB-first not supported on PL022:
    invalid_params_if(SPI, order != SPI_MSB_FIRST);
    invalid_params_if(SPI, cpol != SPI_CPOL_0 && cpol != SPI_CPOL_1);
    invalid_params_if(SPI, cpha != SPI_CPHA_0 && cpha != SPI_CPHA_1);
    hw_write_masked(&spi_get_hw(spi)->cr0,
                    ((uint)(data_bits - 1)) << SPI_SSPCR0_DSS_LSB |
                    ((uint)cpol) << SPI_SSPCR0_SPO_LSB |
                    ((uint)cpha) << SPI_SSPCR0_SPH_LSB,
        SPI_SSPCR0_DSS_BITS |
        SPI_SSPCR0_SPO_BITS |
        SPI_SSPCR0_SPH_BITS);
//New code added        
    hw_clear_bits(&spi_get_hw(spi)->cr1, SPI_SSPCR1_SSE_BITS); //disable the SPI
    hw_set_bits(&spi_get_hw(spi)->cr1, SPI_SSPCR1_SSE_BITS); //re-enable the SPI
}

Corrected code in spi.h ``` static inline void spi_set_format(spi_inst_t *spi, uint data_bits, spi_cpol_t cpol, spi_cpha_t cpha, __unused spi_order_t order) { invalid_params_if(SPI, data_bits < 4 || data_bits > 16); // LSB-first not supported on PL022: invalid_params_if(SPI, order != SPI_MSB_FIRST); invalid_params_if(SPI, cpol != SPI_CPOL_0 && cpol != SPI_CPOL_1); invalid_params_if(SPI, cpha != SPI_CPHA_0 && cpha != SPI_CPHA_1); hw_write_masked(&spi_get_hw(spi)->cr0, ((uint)(data_bits - 1)) << SPI_SSPCR0_DSS_LSB | ((uint)cpol) << SPI_SSPCR0_SPO_LSB | ((uint)cpha) << SPI_SSPCR0_SPH_LSB, SPI_SSPCR0_DSS_BITS | SPI_SSPCR0_SPO_BITS | SPI_SSPCR0_SPH_BITS); //New code added hw_clear_bits(&spi_get_hw(spi)->cr1, SPI_SSPCR1_SSE_BITS); //disable the SPI hw_set_bits(&spi_get_hw(spi)->cr1, SPI_SSPCR1_SSE_BITS); //re-enable the SPI } ```

dhalbert commented

2022-11-17 17:53:41 -06:00

(Migrated from github.com)

I also encountered this on a Mode 3 device (Analog Devices accelerometer). Code above is also discussed here: https://forums.raspberrypi.com/viewtopic.php?t=336142

daveythacher commented

2022-12-26 06:47:10 -06:00

(Migrated from github.com)

This looks related to #713. I submitted a pull request which should work for this also. (#1158)

kilograham commented

2024-07-21 13:02:32 -05:00

(Migrated from github.com)

Sounds like this was actually fixed in 1.5.0; please reopen otherwise

cheng3100 commented

2024-08-19 09:58:47 -05:00

(Migrated from github.com)

Hi @kilograham , I find the same issue with spi slave mode0. I use raspi 4 as spi master. I try the latest pico sdk master&develop branch but the issue still here. spi slave mode1 is fine.

daveythacher commented

2024-08-19 10:24:47 -05:00

(Migrated from github.com)

Please give the SDK version in use?

cheng3100 commented

2024-08-19 21:12:44 -05:00

(Migrated from github.com)

Please give the SDK version in use?

I use the latest github version of the pico-sdk, both master and develop branch has the same issue.
(I see the submit which @daveythacher mentioned above is merged into develop branch so I also try this branch )

> Please give the SDK version in use? I use the latest github version of the pico-sdk, both master and develop branch has the same issue. (I see the submit which @daveythacher mentioned above is merged into develop branch so I also try this branch )

daveythacher commented

2024-08-20 07:45:29 -05:00

(Migrated from github.com)

Is this relevant? I would recommended filing SPI slave issues independently.

Overall I noticed ARM SPI is a little different. Also the SDK interface is mostly based on master mode. Not all SPI hardware features are available in the SDK. I view the SDK methods as helpers rather than a complete API.

Did you try this? (The order could matter.)

{
     spi_init(SPI0, 4000000);
     spi_set_slave(SPI0, true);
     spi_set_format(SPI0, 8, SPI_CPOL_1, SPI_CPHA_1, SPI_MSB_FIRST);
}

Edit:
Any objection to an enable method? That is the only thing spi_init actually does.

Is [this](https://github.com/earlephilhower/arduino-pico/issues/1746#issuecomment-1757946718) relevant? I would recommended filing SPI slave issues independently. Overall I noticed ARM SPI is a little different. Also the SDK interface is mostly based on master mode. Not all SPI hardware features are available in the SDK. I view the SDK methods as helpers rather than a complete API. Did you try this? (The order could matter.) ``` C { spi_init(SPI0, 4000000); spi_set_slave(SPI0, true); spi_set_format(SPI0, 8, SPI_CPOL_1, SPI_CPHA_1, SPI_MSB_FIRST); } ``` Edit: Any objection to an enable method? That is the only thing spi_init actually does.

cheng3100 commented

2024-08-21 22:26:23 -05:00

(Migrated from github.com)

Yes， I just modify a little the pico-example/spi/spi_master_slave/spi_slave.c as below:

61     // Enable SPI 0 at 1 MHz and connect to GPIOs
62     spi_init(spi_default, 1000 * 1000);
63     spi_set_slave(spi_default, true);
64     spi_set_format(spi_default, 8, SPI_CPOL_0, SPI_CPHA_1, SPI_MSB_FIRST);

This is mode1 which work fine. But if I comment the line 64 or modify line 64 as

64     spi_set_format(spi_default, 8, SPI_CPOL_0, SPI_CPHA_0, SPI_MSB_FIRST);

The mode0 will not work.

I also check this but it just say it should be a sdk issue because the arduino-pico lib is just a encapsulation of the pico-sdk I think.

I haven't capture the waveform. But the mode0 behave no matter how many the master clk one time, the slave only return one correct byte one time in the first 8 bit clk position, then the slave return the next one correct byte when I use spi master to send again.

The correct behave of the spi slave example demo should be that it will return BUF_LEN bytes in one time. That also what happen in mode1.

Yes， I just modify a little the pico-example/spi/spi_master_slave/spi_slave.c as below: ``` 61 // Enable SPI 0 at 1 MHz and connect to GPIOs 62 spi_init(spi_default, 1000 * 1000); 63 spi_set_slave(spi_default, true); 64 spi_set_format(spi_default, 8, SPI_CPOL_0, SPI_CPHA_1, SPI_MSB_FIRST); ``` This is mode1 which work fine. But if I comment the line 64 or modify line 64 as ``` 64 spi_set_format(spi_default, 8, SPI_CPOL_0, SPI_CPHA_0, SPI_MSB_FIRST); ``` The mode0 will not work. I also check [this](https://github.com/earlephilhower/arduino-pico/issues/1746#issuecomment-1757946718) but it just say it should be a sdk issue because the arduino-pico lib is just a encapsulation of the pico-sdk I think. I haven't capture the waveform. But the mode0 behave no matter how many the master clk one time, the slave only return one correct byte one time in the first 8 bit clk position, then the slave return the next one correct byte when I use spi master to send again. The correct behave of the spi slave example demo should be that it will return `BUF_LEN` bytes in one time. That also what happen in mode1.

daveythacher commented

2024-08-22 06:21:11 -05:00

(Migrated from github.com)