raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.72k stars 925 forks source link

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

Open UKTailwind opened 2 years ago

UKTailwind commented 2 years ago

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 1 year ago

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 1 year ago

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

kilograham commented 3 months ago

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

cheng3100 commented 2 months ago

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 2 months ago

Please give the SDK version in use?

cheng3100 commented 2 months ago

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 2 months ago

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.

cheng3100 commented 2 months ago

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.

daveythacher commented 2 months ago

I am thinking this is a different issue. Please verify the CS line.