raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.73k stars 926 forks source link

spi_set_format does not support setting Frame Format FRF #1099

Open minameHeart opened 1 year ago

minameHeart commented 1 year ago

Current implementation of spi.h does not support setting Frame Format FRF. According to RP2040 Datasheet following formats are possible:

  1. Motorola SPI frame format. 0x00
  2. TI synchronous serial frame format. 0x01
  3. National Microwire frame format. 0x10

spi_set_format should look something like that

typedef enum {
    SPI_FRF_M  = 0,
    SPI_FRF_TI = 1,
    SPI_FRF_NM = 2
} spi_frf_t;

static inline void spi_set_format(spi_inst_t *spi, uint data_bits, spi_frf_t frf, 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, frf != SPI_FRF_M && frf != SPI_FRF_TI && frf != SPI_FRF_NM);
    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) frf) << SPI_SSPCR0_FRF_LSB | // SETTING BIT
                    ((uint)cpol) << SPI_SSPCR0_SPO_LSB |
                    ((uint)cpha) << SPI_SSPCR0_SPH_LSB,
        SPI_SSPCR0_DSS_BITS |
        SPI_SSPCR0_FRF_BITS | // ADD MASK !!!
        SPI_SSPCR0_SPO_BITS |
        SPI_SSPCR0_SPH_BITS);
}

spi_init() should then have a parameter to select Frame Format (FRF) or there should be at least a function to set Frame Format (FRF). Otherwise users are forced to write directly into register or modify SDK (unclean).

Btw. Datasize (DSS) is fixed to value 8 which makes the function spi_write16_blocking() and spi_write16_read16_blocking() useless. Datasize should be parameter, too.

magy00 commented 3 weeks ago

https://forums.raspberrypi.com/viewtopic.php?p=2259448#p2259448

Maybe the order parameter (currently unused, i.e. constant value 1, or possibly 0) could be repurposed as some combined frame format/mode/order enum (like FRF = order >> 1)…